Closed
Bug 733793
Opened 12 years ago
Closed 12 years ago
Crash in js::CrossCompartmentWrapper::getPropertyDescriptor @ xpc::WrapperFactory::PrepareForWrapping (mainly correlated to Youtube Video Replay)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: scoobidiver, Unassigned)
Details
(4 keywords, Whiteboard: [startupcrash][qa+])
Crash Data
Attachments
(3 files, 1 obsolete file)
1.72 KB,
patch
|
bholley
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
33.71 KB,
application/octet-stream
|
Details | |
33.71 KB,
application/octet-stream
|
Details |
It's a new crash signature that first appeared in 13.0a1/20120223. The regression range is: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=373c710112e6&tochange=5e756e59a794 It might be a regression from bug 717113. Signature xpc::WrapperFactory::PrepareForWrapping(JSContext*, JSObject*, JSObject*, unsigned int) More Reports Search UUID f38d49ed-58b1-4b0d-a0c4-3210c2120307 Date Processed 2012-03-07 13:46:46 Uptime 5 Last Crash 28.9 minutes before submission Install Age 29.2 minutes since version was first installed. Install Time 2012-03-07 14:30:34 Product Firefox Version 13.0a1 Build ID 20120306031101 Release Channel nightly OS Windows NT OS Version 6.1.7600 Build Architecture x86 Build Architecture Info GenuineIntel family 6 model 23 stepping 10 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x0 App Notes AdapterVendorID: 0x8086, AdapterDeviceID: 0x2a42, AdapterSubsysID: 030d1025, AdapterDriverVersion: 8.15.10.2202 D2D? D2D+ DWrite? DWrite+ Processor Notes This dump is too long and has triggered the automatic truncation routine EMCheckCompatibility True Total Virtual Memory 2147352576 Available Virtual Memory 1897250816 System Memory Use Percentage 42 Available Page File 3201490944 Available Physical Memory 1200078848 Frame Module Signature [Expand] Source 0 xul.dll xpc::WrapperFactory::PrepareForWrapping js/xpconnect/wrappers/WrapperFactory.cpp:177 1 mozjs.dll JSCompartment::wrap js/src/jscompartment.cpp:227 2 mozjs.dll JSCompartment::wrap js/src/jscompartment.cpp:349 3 mozjs.dll JSCompartment::wrap js/src/jscompartment.cpp:392 4 mozjs.dll js::CrossCompartmentWrapper::getPropertyDescriptor js/src/jswrapper.cpp:543 5 mozjs.dll js::Proxy::getPropertyDescriptor js/src/jsproxy.cpp:765 6 mozjs.dll js_SetPropertyHelper js/src/jsobj.cpp:5380 7 mozjs.dll js::Wrapper::set js/src/jswrapper.cpp:234 8 mozjs.dll js::CrossCompartmentWrapper::set js/src/jswrapper.cpp:630 9 mozjs.dll js::Proxy::set js/src/jsproxy.cpp:885 10 mozjs.dll proxy_SetProperty js/src/jsproxy.cpp:1122 11 mozjs.dll JSObject::nonNativeSetProperty js/src/jsobj.cpp:3102 12 mozjs.dll js::SetPropertyOperation js/src/jsinterpinlines.h:372 13 mozjs.dll js::mjit::stubs::SetName<0> js/src/methodjit/StubCalls.cpp:106 14 mozjs.dll js::mjit::EnterMethodJIT js/src/methodjit/MethodJIT.cpp:1052 15 mozjs.dll js::mjit::JaegerShot js/src/methodjit/MethodJIT.cpp:1123 16 mozjs.dll js::RunScript js/src/jsinterp.cpp:451 17 mozjs.dll js::InvokeKernel js/src/jsinterp.cpp:517 18 mozjs.dll js::Invoke js/src/jsinterp.cpp:549 19 mozjs.dll js::ProxyHandler::call js/src/jsproxy.cpp:334 20 mozjs.dll js::CrossCompartmentWrapper::call js/src/jswrapper.cpp:736 21 mozjs.dll js::Proxy::call js/src/jsproxy.cpp:909 22 mozjs.dll proxy_Call js/src/jsproxy.cpp:1428 23 mozjs.dll js::InvokeKernel js/src/jsinterp.cpp:492 ... More reports at: https://crash-stats.mozilla.com/report/list?signature=xpc%3A%3AWrapperFactory%3A%3APrepareForWrapping%28JSContext*%2C%20JSObject*%2C%20JSObject*%2C%20unsigned%20int%29 https://crash-stats.mozilla.com/report/list?signature=xpc%3A%3AWrapperFactory%3A%3APrepareForWrapping
Comment 1•12 years ago
|
||
(In reply to Scoobidiver from comment #0) > It's a new crash signature that first appeared in 13.0a1/20120223. > It might be a regression from bug 717113. Not super likely. It looks like it's crashing here: http://hg.mozilla.org/mozilla-central/annotate/7d0d1108a14e/js/xpconnect/wrappers/WrapperFactory.cpp#l177 Hard to say for sure, but there's a good chance obj is null.
Reporter | ||
Comment 2•12 years ago
|
||
It's currently #1 top crasher in 13.0a2. One comment says: "Add On Incompatibility - Appearance" Here are correlations per add-on on March 20: xpc::WrapperFactory::PrepareForWrapping(JSContext*, JSObject*, JSObject*, unsigned int)|EXCEPTION_ACCESS_VIOLATION_READ (30 crashes) 87% (26/30) vs. 7% (27/394) {e1aaa9f8-4500-47f1-9a0a-b02bd60e4076} (Youtube Video Replay) 3% (1/30) vs. 0% (1/394) 0.5 83% (25/30) vs. 7% (26/394) 1.2 xpc::WrapperFactory::PrepareForWrapping|EXC_BAD_ACCESS / KERN_INVALID_ADDRESS (20 crashes) 75% (15/20) vs. 32% (15/47) {c75a27d8-4529-449f-b67b-aba65d7a1c0a} (Toggle Web Developer Toolbar 3.4) 60% (12/20) vs. 26% (12/47) rsDownloader@163.com (RS+MU Downloader 2.4.1) 60% (12/20) vs. 26% (12/47) jsdeobfuscator@adblockplus.org (JavaScript Deobfuscator, https://addons.mozilla.org/addon/10345) (1.6.3) 55% (11/20) vs. 23% (11/47) {dc572301-7619-498c-a57d-39143191b318} (Tab Mix Plus, https://addons.mozilla.org/addon/1122) (0.4.0) 55% (11/20) vs. 23% (11/47) tilt@mozilla.com (Tilt 1.0.1) I installed Youtube Video Replay 1.2 in 12.0b1, then I opened this profile with 13.0a2/20120312 and I got a "InternalError: too much recursion" prompt on first startup and bp-778df487-b91e-4fbf-afd1-d06a72120321 on second startup.
tracking-firefox13:
--- → ?
Keywords: reproducible,
topcrash
Updated•12 years ago
|
Comment 3•12 years ago
|
||
It looks like the crash rate has a distinct downward trend. Could a fix have been landed?
Comment 4•12 years ago
|
||
So I looked a little more. The callstacks definitely seem to be showing an infinite C-stack recursion. Now, JSCompartment::wrap contains JS_CHECK_RECURSION, so blowing the stack probably isn't the cause. Rather, some code that trips JS_CHECK_RECURSION, triggers an error, and then returns NULL. Looking at the crash location: http://hg.mozilla.org/releases/mozilla-aurora/annotate/1eb6d3a4018b/js/xpconnect/wrappers/WrapperFactory.cpp#l177 I see a call to the outerObject hook (which can return null (see XPC_WN_OuterObject)) which doesn't check the return value for null. (This is all in GetCurrentOuter, which I assume is inlined.)
Reporter | ||
Comment 5•12 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #3) > It looks like the crash rate has a distinct downward trend. Could a fix > have been landed? No. 76% of crashes happen within one minute.
Whiteboard: [startupcrash]
Comment 6•12 years ago
|
||
(In reply to Scoobidiver from comment #5) > No. 76% of crashes happen within one minute. I was looking at this graph: http://bit.ly/GVF7PH. "No" doesn't seem to follow from "76% of crashes happen within one minute". Regardless, we should fix the bug in comment 4.
Reporter | ||
Comment 7•12 years ago
|
||
If a user crashes on startup, he won't use Firefox, so once all users that should crash hit it, there are no crashes, but it doesn't mean it's fixed.
Comment 8•12 years ago
|
||
Ah, that makes more sense.
Reporter | ||
Updated•12 years ago
|
Summary: Crash in js::CrossCompartmentWrapper::getPropertyDescriptor @ xpc::WrapperFactory::PrepareForWrapping → Crash in js::CrossCompartmentWrapper::getPropertyDescriptor @ xpc::WrapperFactory::PrepareForWrapping (mainly correlated to Youtube Video Replay)
Comment 9•12 years ago
|
||
Adding Youtube Video Replay developer to CC list.
Comment 10•12 years ago
|
||
What are the next actions on this bug? - Luke, can you write a patch for the bug in comment 4? Do you think that's the cause of these crashes? - Can we learn anything more from the stacks? There are some things that don't make sense to me, but it looks like some kind of property-set operation is going to a proxy, which triggers the same property-set operation? - Baris, does that sound familiar at all? Are you using proxies, or is it something internal to Firefox? Speaking of which, do you ever get this crash in testing? - Luke, would it make sense to add a diagnostic to check for this recursive operation to make it fail faster with a better error message?
Comment 11•12 years ago
|
||
(In reply to David Mandelin from comment #10) > - Luke, can you write a patch for the bug in comment 4? Sure, I was waiting for an xpconnect person to comment on this, but I guess it's pretty simple. > Do you think that's the cause of these crashes? The return value (NULL) and source location both match the crash address. > - Luke, would it make sense to add a diagnostic to check for this > recursive operation to make it fail faster with a better error message? Well, that's what JS_CHECK_RECURSION is. Assuming xpconnect doesn't crash, you get the whole callstack in the exception.
Comment 12•12 years ago
|
||
Attachment #610329 -
Flags: review?(bobbyholley+bmo)
Updated•12 years ago
|
Attachment #610329 -
Flags: review?(bobbyholley+bmo) → review+
Comment 13•12 years ago
|
||
Actually two are needed.
Attachment #610350 -
Flags: review?(bobbyholley+bmo)
Updated•12 years ago
|
Attachment #610350 -
Flags: review?(bobbyholley+bmo) → review+
Comment 14•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ed31daf07bd
Target Milestone: --- → mozilla14
Updated•12 years ago
|
Attachment #610329 -
Attachment is obsolete: true
Comment 15•12 years ago
|
||
Comment on attachment 610350 [details] [diff] [review] null check x2 [Approval Request Comment] Risk to taking this patch (and alternatives if risky): low, null check
Attachment #610350 -
Flags: approval-mozilla-aurora?
Comment 16•12 years ago
|
||
- David, I am not good at Firefox Internals but to me it somehow relates to AddonsManager and mozIJSSubScriptLoader.loadSubScript
Comment 17•12 years ago
|
||
- David, I am not good at Firefox Internals but to me it somehow relates to AddonsManager and mozIJSSubScriptLoader. When trying the attached YVR.withAddonsManager.xpi, the line 37 in youtubevideoreplay_bootstrap.js: AddonManager.getAddonByID(YoutubeVideoReplay.addonGUID, function(addon) { was creating the Error Prompt. When trying the attached YVR.noAddonsManager.xpi which removed the AddonManager line the error is not thrown anymore. Now and then, I see the below warning in Error Console: Warning: anonymous function does not always return a value Source file: resource://app/modules/XPIProvider.jsm -> file:///Users/barisderin/Library/Application%20Support/Firefox/Profiles/7gy2pfh7.YVR%201/extensions/%7Be1aaa9f8-4500-47f1-9a0a-b02bd60e4076%7D/bootstrap.js -> file:///Users/barisderin/Library/Application%20Support/Firefox/Profiles/7gy2pfh7.YVR%201/extensions/%7Be1aaa9f8-4500-47f1-9a0a-b02bd60e4076%7D/chrome/content/youtubevideoreplay_bootstrap.js Line: 45 - Jorge, to protect FF 13 users I am going to remove Bootstrap support and switch to Restart version for the add-on till the bug is fixed. Is it OK?
Comment 18•12 years ago
|
||
Comment 19•12 years ago
|
||
Comment 20•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7ed31daf07bd
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 21•12 years ago
|
||
Oops, I should have added [leave open after merge]. I'll reopen so we can continue to track the issue and see if the patch fixes the topcrash.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 22•12 years ago
|
||
(In reply to Baris Derin from comment #17) > - Jorge, to protect FF 13 users I am going to remove Bootstrap support and > switch to Restart version for the add-on till the bug is fixed. Is it OK? Yeah, that's fine. Just mention that in the Notes to Reviewer so that we know why you're doing that.
Comment 23•12 years ago
|
||
Comment on attachment 610350 [details] [diff] [review] null check x2 [Triage Comment] Low risk addition of a null check - approved for Aurora 13.
Attachment #610350 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reporter | ||
Comment 25•12 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #21) > Oops, I should have added [leave open after merge]. I'll reopen so we can > continue to track the issue and see if the patch fixes the topcrash. In the trunk, there are no crashes after 14.0a1/20120329 on Windows and 14.0a1/20120326 on Mac/Linux. As the patch landed in 14.0a1/20120330, it's probably fixed.
Updated•12 years ago
|
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•12 years ago
|
status-firefox13:
--- → fixed
status-firefox14:
--- → verified
Comment 26•12 years ago
|
||
qa+ for verification by checking crashstats.
Whiteboard: [startupcrash] → [startupcrash][qa+]
Comment 27•12 years ago
|
||
http://bit.ly/Kcn9aV Fixed for 13 - only around 15 crashes cumulated for all 13 beta But around *6000* crashes for 15a1 Nightly with the same signature in the last 2 weeks. Don't see this tracked in a separate bug by watching the Bugzilla tab in Socorro. Would it make sense to log a new issue for that or keep investigating here?
Comment 28•12 years ago
|
||
(In reply to Virgil Dicu [:virgil] [QA] from comment #27) > Would it make sense to log a new issue for that or keep investigating here? If this is different than bug 752309, then we should definitely file a new bug for it. Almost certainly related to compartment-per-global.
Comment 29•12 years ago
|
||
I couldn't reproduce bug 752309 (WFM for several people) so logged bug 757003 for the Firefox 15 crashes. Marking this as verified in 13 (only 15 cumulated beta crashes)
You need to log in
before you can comment on or make changes to this bug.
Description
•