Closed Bug 1204324 Opened 9 years ago Closed 9 years ago

crash in nsXMLHttpRequest::Send(nsIVariant*, mozilla::dom::Nullable<T> const&) rising in 41.0b9

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Version:
41 Branch
Platform:
x86
Windows NT
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox41 + fixed

People

(Reporter: philipp, Unassigned)

References

Details

(Keywords: crash)

Crash Data

This bug was filed from the Socorro interface and is report bp-027c23e1-4696-44f3-a423-b52b72150913. ============================================================= 0 xul.dll nsXMLHttpRequest::Send(nsIVariant*, mozilla::dom::Nullable<nsXMLHttpRequest::RequestBody> const&) dom/base/nsXMLHttpRequest.cpp 1 xul.dll nsXMLHttpRequest::Send(nsXMLHttpRequest::RequestBody const&) dom/base/nsXMLHttpRequest.h 2 xul.dll nsXMLHttpRequest::Send(JSContext*, nsAString_internal const&, mozilla::ErrorResult&) dom/base/nsXMLHttpRequest.h 3 xul.dll mozilla::dom::XMLHttpRequestBinding::send obj-firefox/dom/bindings/XMLHttpRequestBinding.cpp 4 xul.dll mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) dom/bindings/BindingUtils.cpp 5 xul.dll js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp 6 xul.dll js::TypeScript::Monitor(JSContext*, JSScript*, unsigned char*, JS::Value const&) js/src/vm/TypeInference-inl.h 7 xul.dll Interpret js/src/vm/Interpreter.cpp 8 xul.dll js::frontend::BytecodeEmitter::bindNameToSlot(js::frontend::ParseNode*) js/src/frontend/BytecodeEmitter.cpp 9 xul.dll js::frontend::BytecodeEmitter::emitNameOp(js::frontend::ParseNode*, bool) js/src/frontend/BytecodeEmitter.cpp 10 xul.dll js::frontend::BytecodeEmitter::emitCallOrNew(js::frontend::ParseNode*) js/src/frontend/BytecodeEmitter.cpp 11 xul.dll js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*) js/src/frontend/BytecodeEmitter.cpp 12 xul.dll Evaluate js/src/jsapi.cpp 13 xul.dll nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) dom/base/nsJSUtils.cpp 14 xul.dll nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) dom/base/nsJSUtils.cpp 15 xul.dll nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, JS::SourceBufferHolder&, void**) dom/base/nsScriptLoader.cpp this crash is on the rise after the 41.0b9 release. at the moment it takes place #1 in the top score board with 7 crashes per installation and 71% during startup. when looking through the reports, this crash seems correlated to the presence of one of those extensions most of the time: {d49175b3-3fd8-43b8-b28e-da5d47f3c398} 1.0.59 COMPUTERBILD-Abzockschutz firefox-support@vworldc.com vb-20150728103734 VWC Cocoon
[Tracking Requested - why for this release]: As philipp says, this is #1 in Top Crash Score as it happens to ~70% at startup with 3.6 crashes per installation, so really annoying to users.
tracking-firefox41: --- → ?
uploadChannel is null, meaning that uploadChannel2 must have also been null, meaning that this would have triggered on a debug build: // This assertion will fire if buggy extensions are installed NS_ASSERTION(uploadChannel2, "http must support nsIUploadChannel2"); Could this be related to the disabled addon signing requirement in b9?
> correlated to the presence of one of those extensions most of the time: > {d49175b3-3fd8-43b8-b28e-da5d47f3c398} 1.0.59 COMPUTERBILD-Abzockschutz Our correlations files are broken again, but in a spot-check I do see this 1.0.59 extension frequently. And other data generally agrees: I see 74% "de" locale, and some of the top URLs are computerbild.de and amazon.de.
the other mentioned addon that might be related to this crash is more prevalent in en-us locales firefox-support@vworldc.com vb-20150728103734 VWC Cocoon i could reproduce this crash with "COMPUTERBILD-Abzockschutz 1.0.59" on beta 41.0b9 but not with 40.0.3 - all that was necessary to trigger it was a visit amazon.de, which causes a crash loop after the browser is restarted as well. the regression range is https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8e1657e7e7433a561c6e84d3820dc16471647158&tochange=4819768c871f5f64c5e2a984c7794f5385bc2977
> the regression range is > https://hg.mozilla.org/integration/mozilla-inbound/ > pushloghtml?fromchange=8e1657e7e7433a561c6e84d3820dc16471647158&tochange=4819 > 768c871f5f64c5e2a984c7794f5385bc2977 Thanks Philipp! I have a feeling that this is an ordinary case of "codebase changed and old addons can't deal with it" but I'd like to give Christoph a chance to weigh in before we block these addons. Do you see any indication that this bug is unusual?
Flags: needinfo?(mozilla)
(In reply to David Major [:dmajor] from comment #5) > I have a feeling that this is an ordinary case of "codebase changed and old > addons can't deal with it" but I'd like to give Christoph a chance to weigh > in before we block these addons. Do you see any indication that this bug is > unusual? If this line [1] would fire in a debug build, then I suppose it's fine to disable those addons. Jonas wrote that code, he is probably in a better position to answer your question. So I am deferring to him. [1] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsXMLHttpRequest.cpp#2762
Flags: needinfo?(jonas)
Also clearing my needinfo request.
Flags: needinfo?(mozilla)
Jorge, there are at least two addons that may not be blocked before we gtb 41 RC in the next hour or so. Would you be able to update the addon blocklist and nominate a patch for uplift to Beta/m-r? Many thanks!
Flags: needinfo?(jorge)
the crash is reproducible when installing the addon available from https://www.getcocoon.com/support/download (firefox-support@vworldc.com vb-20150728103734) on 41.0b9 as well: https://crash-stats.mozilla.com/report/index/ac0e999c-4157-4583-abcf-6453b2150914
Jorge - to summarize, it's these two: {d49175b3-3fd8-43b8-b28e-da5d47f3c398} 1.0.59 firefox-support@vworldc.com vb-20150728103734 Philipp was able to reproduce crashes with each of those. We should probably also reach out to that support address.
Okay, I marked both add-ons as incompatible with 41 and above.
Flags: needinfo?(jorge)
Tracking this bug for 41 RC week so we can keep an eye on any/all other addons that might similarly be impacting firefox stability.
status-firefox41: --- → affected
tracking-firefox41: ? → +
After some discussion on IRC it's been decided that it's best to go with the blocklist than the compatibility override, since the startup crashes might only be properly prevented with the blocklist that the user has in the profile or built into Firefox.
There's very little information here to go on to figure out what's going wrong. Actually, I think the problem here is that we need to make nsSecCheckWrapChannelBase implement nsIUploadChannel and nsIUploadChannel2. Alternatively the addon could implement nsIProtocolHandler.newChannel2, but that also means that the addon needs to take on the responsibility to do various security checks, which is not easy to do currently. Christoph, don't we also need to make nsSecCheckWrapChannelBase implement AsyncOpen2?
bhearsum, jlund: Given that the addons blocklist was updated, I was told that RelEng will need to an update on their infra to pick up this change before I gtb 41 RC. Thanks for your help guys!
Flags: needinfo?(jlund)
Flags: needinfo?(bhearsum)
The blocklist has been updated just now.
(In reply to Jonas Sicking (:sicking) from comment #15) > Christoph, don't we also need to make nsSecCheckWrapChannelBase implement > AsyncOpen2? Currently, we only perform call forwarding [1]. So if the underlying channel does not implement asyncOpen2(), then an error is returned. I think we should implement that wrapping functionality, similar to what we do for ::GetLoadInfo(). Jonas, I think we could get the loadInfo from the outer channel and call into the contentSecurityManager ourselves, right? Should we go ahead and do that? [1] http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsSecCheckWrapChannel.h#48
(In reply to Ritu Kothari (:ritu) from comment #16) > bhearsum, jlund: Given that the addons blocklist was updated, I was told > that RelEng will need to an update on their infra to pick up this change > before I gtb 41 RC. Thanks for your help guys! Jordan is taking care of this.
Flags: needinfo?(bhearsum)
I filed Bug 1204648 wrap AsyncOpen2() calls.
Depends on: 1204648
(In reply to Ben Hearsum (:bhearsum) from comment #20) > (In reply to Ritu Kothari (:ritu) from comment #16) > > bhearsum, jlund: Given that the addons blocklist was updated, I was told > > that RelEng will need to an update on their infra to pick up this change > > before I gtb 41 RC. Thanks for your help guys! > > Jordan is taking care of this. I had to trigger the job that updates blocklist twice as the first time it thought there was no changes. there may have been a caching race condition when the first one polled addons.m.o this should be done now: https://hg.mozilla.org/releases/mozilla-release/rev/5b7b83db947a
Flags: needinfo?(jlund)
Flags: qe-verify+
QA Contact: vasilica.mihasca
Now that bug 1204648 has landed, can anyone check if these addons work better? If they are still crashing, then a stack would be most helpful.
Flags: needinfo?(jonas)
(In reply to Jonas Sicking (:sicking) from comment #23) > Now that bug 1204648 has landed, can anyone check if these addons work > better? If they are still crashing, then a stack would be most helpful. We won't see any effects this quickly. The nightly and aurora populations generally don't have these kinds of buggy addons. (And unsigned addons are still disabled on 43 anyway, no?)
Comment 10 made it sound like the crash had been reproduced locally. So I was hoping that we could re-test the same steps on a build after bug 1204648 had landed. Though I'm seeing now that comment 10 wasn't referring to nightly builds, so maybe that would be a bunch of work still.
yes, the crashes are easily reproducible with the mentioned addons in nightly as well. i'll test it with tomorrow's nightly again and report back... (adding note to self)
Flags: needinfo?(madperson)
i tried it with the tinderbox build by dmajor's recommendation and it no longer crashes with any of the three extensions mentioned before, now that bug 1204648 had landed.
Flags: needinfo?(madperson)
Potentially we could uplift bug 1204648 then. I think the patch is pretty safe.
(In reply to Jonas Sicking (:sicking) from comment #28) > Potentially we could uplift bug 1204648 then. I think the patch is pretty > safe. Yep, already did: https://bugzilla.mozilla.org/show_bug.cgi?id=1204648#c9
I just realized, I don't understand why bug 1204648 fixes the bug, if that patch only applies to 42 and 43? Can it not be uplifted to 41?
Jonas, perhaps David's question in comment 30 is directed at you.
Flags: needinfo?(jonas)
It should apply fine to 41 as well, but Christoph should confirm as it is his patch.
Flags: needinfo?(jonas) → needinfo?(mozilla)
(In reply to Jonas Sicking (:sicking) from comment #32) > It should apply fine to 41 as well, but Christoph should confirm as it is > his patch. Well, in Bug 1143922 we added asyncOpen2 to nsIChannel, which landed in 42. Which means that we can *not* uplift to 41. The patch of Bug 1204648 shouldn't even compile in 41 and there shouldn't be any issue in 41. So at the moment I don't quite understand what the problem in 41 was, but it seems it's not related to the problem we fixed in Bug 1204648.
Flags: needinfo?(mozilla)
After uplifting the fix from Bug 1205410, I want to call this "fixed". Please let me know if there are any concerns.
status-firefox41: affected → fixed
(In reply to Ritu Kothari (:ritu) from comment #34) > After uplifting the fix from Bug 1205410, I want to call this "fixed". > Please let me know if there are any concerns. The fix in Bug 1205410 does not fix the problem described in this patch. The right patch for *41* to fix the problem described here would be this patch: https://bugzilla.mozilla.org/show_bug.cgi?id=1204648#c18 which was *not* selected for 41.
Flags: needinfo?(rkothari)
Umm, as far as I understood, this bug was filed for the crash in 41, and bug 1205410 fixes that crash in 41, doesn't it?
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #36) > Umm, as far as I understood, this bug was filed for the crash in 41, and bug > 1205410 fixes that crash in 41, doesn't it? Sorry, my bad. You are absolutely right Robert. I was confused. Since we basically disabled the channel wrapper for 41, we are not facing the problem described in this bug for 41. Sorry for the confusion.
Flags: needinfo?(rkothari)
marking this bug as fixed then, also based on the available crash stats (it's basically gone since rc3 and in 41.0)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.