Last Comment Bug 733793 - Crash in js::CrossCompartmentWrapper::getPropertyDescriptor @ xpc::WrapperFactory::PrepareForWrapping (mainly correlated to Youtube Video Replay)
: Crash in js::CrossCompartmentWrapper::getPropertyDescriptor @ xpc::WrapperFac...
Status: RESOLVED FIXED
[startupcrash][qa+]
: crash, regression, reproducible, topcrash
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 13 Branch
: All All
: -- critical (vote)
: mozilla14
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-07 08:55 PST by Scoobidiver (away)
Modified: 2012-05-21 04:10 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
verified


Attachments
null check (1.12 KB, patch)
2012-03-28 15:56 PDT, Luke Wagner [:luke]
bobbyholley: review+
Details | Diff | Splinter Review
null check x2 (1.72 KB, patch)
2012-03-28 16:16 PDT, Luke Wagner [:luke]
bobbyholley: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Youtube Video Replay with Addons Manager (33.71 KB, application/octet-stream)
2012-03-29 07:03 PDT, Baris Derin
no flags Details
Youtube Video Replay No Addons Manager (33.71 KB, application/octet-stream)
2012-03-29 07:05 PDT, Baris Derin
no flags Details

Description Scoobidiver (away) 2012-03-07 08:55:31 PST
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 Bobby Holley (:bholley) (busy with Stylo) 2012-03-07 16:20:41 PST
(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.
Comment 2 Scoobidiver (away) 2012-03-21 02:06:28 PDT
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.
Comment 3 Luke Wagner [:luke] 2012-03-23 16:49:28 PDT
It looks like the crash rate has a distinct downward trend.  Could a fix have been landed?
Comment 4 Luke Wagner [:luke] 2012-03-23 17:10:34 PDT
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.)
Comment 5 Scoobidiver (away) 2012-03-24 02:58:25 PDT
(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.
Comment 6 Luke Wagner [:luke] 2012-03-24 10:44:01 PDT
(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.
Comment 7 Scoobidiver (away) 2012-03-24 10:51:43 PDT
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 Luke Wagner [:luke] 2012-03-24 10:57:15 PDT
Ah, that makes more sense.
Comment 9 Jorge Villalobos [:jorgev] 2012-03-28 12:01:00 PDT
Adding Youtube Video Replay developer to CC list.
Comment 10 David Mandelin [:dmandelin] 2012-03-28 15:09:05 PDT
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 Luke Wagner [:luke] 2012-03-28 15:52:42 PDT
(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 Luke Wagner [:luke] 2012-03-28 15:56:11 PDT
Created attachment 610329 [details] [diff] [review]
null check
Comment 13 Luke Wagner [:luke] 2012-03-28 16:16:32 PDT
Created attachment 610350 [details] [diff] [review]
null check x2

Actually two are needed.
Comment 15 Luke Wagner [:luke] 2012-03-28 22:59:43 PDT
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
Comment 16 Baris Derin 2012-03-29 06:47:23 PDT
- David, I am not good at Firefox Internals but to me it somehow relates to AddonsManager and mozIJSSubScriptLoader.loadSubScript
Comment 17 Baris Derin 2012-03-29 07:02:24 PDT
- 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 Baris Derin 2012-03-29 07:03:49 PDT
Created attachment 610529 [details]
Youtube Video Replay with Addons Manager
Comment 19 Baris Derin 2012-03-29 07:05:05 PDT
Created attachment 610530 [details]
Youtube Video Replay No Addons Manager
Comment 20 Matt Brubeck (:mbrubeck) 2012-03-29 08:40:52 PDT
https://hg.mozilla.org/mozilla-central/rev/7ed31daf07bd
Comment 21 Luke Wagner [:luke] 2012-03-29 08:51:09 PDT
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.
Comment 22 Jorge Villalobos [:jorgev] 2012-03-29 09:13:04 PDT
(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 Alex Keybl [:akeybl] 2012-03-29 17:42:39 PDT
Comment on attachment 610350 [details] [diff] [review]
null check x2

[Triage Comment]
Low risk addition of a null check - approved for Aurora 13.
Comment 25 Scoobidiver (away) 2012-04-02 11:27:44 PDT
(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.
Comment 26 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-04-26 13:38:17 PDT
qa+ for verification by checking crashstats.
Comment 27 Virgil Dicu [:virgil] [QA] 2012-05-21 03:02:23 PDT
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 Bobby Holley (:bholley) (busy with Stylo) 2012-05-21 03:12:26 PDT
(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 Virgil Dicu [:virgil] [QA] 2012-05-21 04:10:26 PDT
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)

Note You need to log in before you can comment on or make changes to this bug.