620984 - Firefox 4.0b8 Crash Report [@ DEBUG_CheckWrapperThreadSafety(XPCWrappedNative const*) ]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Marcia Knous [:marcia]]

Seen while reviewing early B8 crash stats. http://tinyurl.com/27yoj4e to the crashes so far which are all Windows.

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	DEBUG_CheckWrapperThreadSafety 	js/src/xpconnect/src/xpcwrappednative.cpp:3761
1 	xul.dll 	XPCCallContext::Init 	js/src/xpconnect/src/xpccallcontext.cpp:197
2 	xul.dll 	XPC_WN_NoHelper_Resolve 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:750
3 	mozjs.dll 	CallResolveOp 	js/src/jsobj.cpp:4750
4 	mozjs.dll 	js_GetPropertyHelper 	js/src/jsobj.cpp:5255
5 	mozjs.dll 	js_GetMethod 	js/src/jsobj.cpp:5291

Comment 1

[chris hofmann]

#3 topcrash on beta 8 as number of users starts to ramp up.

Comment 2

[Marcia Knous [:marcia]]

Currently this is the #1 crash on Beta 8. All seven comment appears to be in German. Here are the addon correlations:

100% (349/350) vs.   2% (379/21334) {d49175b3-3fd8-43b8-b28e-da5d47f3c398}
51% (177/350) vs.  12% (2617/21334) {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d} (Adblock Plus, https://addons.mozilla.org/addon/1865)
17% (58/350) vs.   2% (481/21334) {a0d7ccb3-214d-498b-b4aa-0e8fda9a7bf7} (WOT, https://addons.mozilla.org/addon/3456)
9% (33/350) vs.   1% (135/21334) [e-mail] (German Dictionary, https://addons.mozilla.org/addon/3077)
98% (344/350) vs.  91% (19382/21334) {972ce4c6-7e08-4474-a285-3208198ce6fd} (Default, https://addons.mozilla.org/addon/8150)
15% (51/350) vs.   9% (1856/21334) {CAFEEFAC-0016-0000-0022-ABCDEFFEDCBA}

Comment 3

[chris hofmann]

Anyone on this?  It would be a good one to get for beta9 to erase the regression thats blown up after the release of beta8.   It was intermittent on trunk first seen around nov 25th,  but now its a high volume, #2 two crash on beta8

         DEBUG_CheckWrapperThreadSafety.XPCWrappedNative.const..
date     total    breakdown by build
         crashes  count build, count build, ...
20101122   
20101123   
20101124   
20101125 1 4.0b8pre2010112404 	1 , 
20101126   
20101127   
20101128 1 4.0b8pre2010112803 	1 , 
20101129 1 4.0b8pre2010112903 	1 , 
20101130 
20101201   
20101202 1 4.0b8pre2010120103 	1 , 
20101203   
20101204   
20101205   
20101206   
20101207   
20101208   
20101209   
20101210   
20101211   
20101212   
20101213 1 4.0b8pre2010121203 	1 , 
20101214   
20101215 1 4.0b9pre2010121503 	1 , 
20101216 1 4.0b8pre2010121203 	1 , 
20101217   
20101218   
20101219 1 4.0b9pre2010121803 	1 , 
20101220   
20101221 23 4.0b82010121417 	23 , 
20101222 109  	108 4.0b82010121417, 
        		1 4.0b9pre2010122203, 
20101223 220  	215 4.0b82010121417, 
        		4 4.0b9pre2010122203, 	1 4.0b9pre2010122303, 
20101224 242 4.0b82010121417 	242 , 
20101225 318  	317 4.0b82010121417, 
        		1 4.0b8pre2010112611, 
20101226 415  	411 4.0b82010121417, 
        		2 4.0b9pre2010122603, 	2 4.0b9pre2010122503, 
20101227 481 4.0b82010121417 	481 , 
20101228 439 4.0b82010121417 	439 ,

Comment 4

[Brendan Eich [:brendan]]

I enabled DEBUG_CheckWrapperThreadSafety to smoke out add-ons using JS across threads. Boris thought it would not recover and it seems he's right, or (need a debug build) perhaps it's just crashing due to some easy bug in the code.

But the upshot is that some add-on(s) use XPConnect unsafely across threads. We need to find and block them, and evangelize their authors to do something else.

/be

Comment 5

[Brendan Eich [:brendan]]

Are all the crashes in the main thread?

Why can't we get a backtrace past js_GetMethod?

/be

Comment 6

[Brendan Eich [:brendan]]

Look for add-ons with binary components, since gal blocked JS thread manager thread-crossing usage.

/be

Comment 7

[chris hofmann]

running some queries to get a more comprehensive list, but here is a look at a sample of 75 crashes.  

Some of this might be easy to fix since the crashes are coming from lab addons.  The test pilot and NO_ADDONS_FOUND also need a closer look since test pilot ships with beta8.  It's either violating the thread safety rules, or possibly just around as an innocent bystander when the crash happens and the the addon list is gathered up and sent in with the crash report.  this could also be the case for others on the list.

once we get this labs addons sorted out it will be easier to spot other possible violators.

  75 total reports on this sample

  62 "http://addons.mozilla.org/en-US/firefox/addon/13661">Test Pilot
  30 "http://addons.mozilla.org/en-US/firefox/addon/1865">Adblock Plus
  10 "http://addons.mozilla.org/en-US/firefox/addon/3456">Web of Trust - Safe Browsing Tool
   7 "http://addons.mozilla.org/en-US/firefox/addon/9449">Microsoft .NET Framework Assistant
   4 NO_ADDONS_FOUND
   4 "http://addons.mozilla.org/en-US/firefox/addon/6249">Google Toolbar
   3 "http://addons.mozilla.org/en-US/firefox/addon/220">FlashGot
   2 "http://addons.mozilla.org/en-US/firefox/addon/6543">Nightly Tester Tools
   2 "http://addons.mozilla.org/en-US/firefox/addon/5971">PrintPDF
   2 "http://addons.mozilla.org/en-US/firefox/addon/5203">Minimap Sidebar
   2 "http://addons.mozilla.org/en-US/firefox/addon/4810">Speed Dial
   2 "http://addons.mozilla.org/en-US/firefox/addon/2410">Xmarks Sync
   2 "http://addons.mozilla.org/en-US/firefox/addon/2324">Session Manager
   2 "http://addons.mozilla.org/en-US/firefox/addon/15003">Add-on Compatibility Reporter
   2 "http://addons.mozilla.org/en-US/firefox/addon/1191">ReminderFox
   2 "http://addons.mozilla.org/en-US/firefox/addon/10137">Easy YouTube Video Downloader
   1 "http://addons.mozilla.org/en-US/firefox/addon/9661">Coupons24.com
   1 "http://addons.mozilla.org/en-US/firefox/addon/918">gTranslate
   1 "http://addons.mozilla.org/en-US/firefox/addon/8879">FoxTab
   1 "http://addons.mozilla.org/en-US/firefox/addon/8542">LastPass Password Manager
   1 "http://addons.mozilla.org/en-US/firefox/addon/7661">Read It Later
   1 "http://addons.mozilla.org/en-US/firefox/addon/748">Greasemonkey
   1 "http://addons.mozilla.org/en-US/firefox/addon/6623">BetterPrivacy
   1 "http://addons.mozilla.org/en-US/firefox/addon/57532">LavaFox V1-Green
   1 "http://addons.mozilla.org/en-US/firefox/addon/5709">YouPlayer
   1 "http://addons.mozilla.org/en-US/firefox/addon/5579">Cooliris
   1 "http://addons.mozilla.org/en-US/firefox/addon/4925">AutoPager
   1 "http://addons.mozilla.org/en-US/firefox/addon/4882">Tab Scope
   1 "http://addons.mozilla.org/en-US/firefox/addon/46375">GutscheinRausch.de
   1 "http://addons.mozilla.org/en-US/firefox/addon/4490">WebMail Notifier (for Gmail, Hotmail, Yahoo, AOL and so on)
   1 "http://addons.mozilla.org/en-US/firefox/addon/4364">Element Hiding Helper for Adblock Plus
   1 "http://addons.mozilla.org/en-US/firefox/addon/3905">Exif Viewer
   1 "http://addons.mozilla.org/en-US/firefox/addon/261477">Mozilla Labs: Prospector - Instant Preview
   1 "http://addons.mozilla.org/en-US/firefox/addon/252539">F1 by Mozilla Labs
   1 "http://addons.mozilla.org/en-US/firefox/addon/244868">Mozilla Labs: Lab Kit
   1 "http://addons.mozilla.org/en-US/firefox/addon/243488">Mozilla Labs: Prospector - Find Suggest
   1 "http://addons.mozilla.org/en-US/firefox/addon/242706">Mozilla Labs: Prospector - Speak Words
   1 "http://addons.mozilla.org/en-US/firefox/addon/2325">RSS Ticker
   1 "http://addons.mozilla.org/en-US/firefox/addon/191969">BlackFox V1-Blue
   1 "http://addons.mozilla.org/en-US/firefox/addon/1810">Showcase
   1 "http://addons.mozilla.org/en-US/firefox/addon/1368">ColorfulTabs
   1 "http://addons.mozilla.org/en-US/firefox/addon/125440">Facebook PhotoZoom
   1 "http://addons.mozilla.org/en-US/firefox/addon/11869">Download YouTube Videos as MP4 and FLV
   1 "http://addons.mozilla.org/en-US/firefox/addon/10909">IE Tab Plus (FF 3.6+)
   1 "http://addons.mozilla.org/en-US/firefox/addon/10900">Personas Plus
   1 "http://addons.mozilla.org/en-US/firefox/addon/10868">Firefox Sync

Comment 8

[chris hofmann]

Attachment: inventory of addon info for all crashes with this signature on jan 07

Comment 9

[Brendan Eich [:brendan]]

I don't think TestPilot uses threads -- cc'ing Jono to confirm.

Cc'ing Jorge too. IIRC we had a list of add-ons with binary components. Would be great to cross-index against comment 7.

/be

Comment 10

[Brendan Eich [:brendan]]

Hmmm:

"http://addons.mozilla.org/en-US/firefox/addon/9449">Microsoft .NET Framework Assistant

from attachment 502259 [details] (comment 8).

/be

Comment 11

[chris hofmann]

Attachment: better list with addons not served off amo

ah, this could get harder.  several of these in the top slots are addons not served off amo, with no contact info.  we'll have to look at uuid's and try and figure out what they are, and who provides them, with some google searches.

Comment 12

[chris hofmann]

Attachment: summary listing with counts of addons spotted on jan. 7

Comment 14

[Boris Zbarsky [:bzbarsky]]

So...  In this case, we're consistently crashing at 0x0.  |wrapper| is known to be non-null (the callsite checked it, in fact, in the cases I looked at).  So either wrapper->mThread is null (how?) or somehow wrapper->mThread's vtable pointer is null.  

That's assuming we trust our line numbers and crash addresses, which I mostly do.

Note that mThread is set in XPCWrappedNative::FinishInit, which I would certainly hope has been checked by that point.

Comment 15

[(not currently active) Ted Mielczarek]

(In reply to comment #5)
> Why can't we get a backtrace past js_GetMethod?

I'm not really sure yet. I poked at it a little bit using this crash report:
https://crash-stats.mozilla.com/report/index/bc6c59a6-870b-4e51-b6e9-0d73a2110110

Without having the binaries available locally, just the symbols, and loading it in WinDBG, the stack was even shorter than ours, so that's not helpful. After downloading the binaries, it's able to unwind all the way (huh, I wonder what it's doing smarter than us?) I'll attack a full stack in a minute.


Unrelated to all that, while poking this minidump in the debugger, I noticed that it has mzvkbd3.dll installed, which is some Kaspersky thing. chofmann: it might be worthwhile checking correlations on that.

Comment 16

[(not currently active) Ted Mielczarek]

Attachment: full stack

Comment 17

[Scoobidiver (away)]

> it might be worthwhile checking correlations on that.
Correlations are automatically generated by dbaron's script.
For mzvkbd3.dll: 24% vs 4%.

May be related to German because all comments are in German.
Comments says (translated from Google):
"Crash-message always comes when you close Firefox beta 8"
"have only the windows shut with x and then this error message!"

Comment 18

[chris hofmann]

(In reply to comment #15)
...
> Unrelated to all that, while poking this minidump in the debugger, I noticed
> that it has mzvkbd3.dll installed, which is some Kaspersky thing. chofmann: it
> might be worthwhile checking correlations on that.

I looked at a sample of 100 reports for jan 7.  mzvkbd3.dll is around about 32% of the time, and version distribution is broad, so if there is a thread safety problem there it may have been around since early versions.

  68  
  18 mzvkbd3.dll 9.0.0.747
   4 mzvkbd3.dll 9.0.0.740
   4 mzvkbd3.dll 11.0.2.556
   4 mzvkbd3.dll 11.0.1.400
   2 mzvkbd3.dll 8.0.0.523
   1 mzvkbd3.dll 9.0.0.464

Do we have a "Binary Addons - Thread Saftey Do's and Don't's" document around somewhere?   I have some contacts from Kaspersky that might be helpful in checking their code for problems.

Comment 19

[Brendan Eich [:brendan]]

We do not want binary add-on components touching main-thread-only objects at all. Andreas: even without the DEBUG_* junk, do we have that checked now?

/be

Comment 20

[Andreas Gal :gal]

No, I don't think we check that in opt builds. JS access is checked, but not access from native code extensions.

Comment 21

[Brendan Eich [:brendan]]

(In reply to comment #20)
> No, I don't think we check that in opt builds. JS access is checked, but not
> access from native code extensions.

Any reason why we shouldn't check from C++ too?

/be

Comment 22

[Brendan Eich [:brendan]]

Talked to jst and bent, bent is kindly taking this one.

/be

Comment 23

[Brendan Eich [:brendan]]

(In reply to comment #19)
> We do not want binary add-on components touching main-thread-only objects at
> all.

I meant from non-main threads, of course.

/be

Comment 24

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

So the stack in attachment 502501 [details] shows something pretty disturbing. We're doing a lot of stuff with XPCOM services after we've shut down the thread manager and the service manager.

I don't know how to prevent that in a simple or safe manner for Firefox 4. We need to take a long look at our shutdown process (specifically the interaction between the cycle collector and XPConnect and JS components that may be used as services) after we ship.

For now we can simply prevent the crash by switching away from using nsIThread[Manager] to simply using NSPR.

mThread is null in this case because the wrapper was created after the thread manager was shut down. After that time it will refuse to hand out anything for NS_GetCurrentThread and friends, even if we're on the main thread (as we are in the stack above).

NS_IsMainThread has been made to work even after the thread manager has been shut down, so we can continue to use that.

Comment 25

[chris hofmann]

not many comments overall since the first of the year, but most are about shutting down, and just about all are in German for some reason.  here are comments not about shut down.

    1 How do I get this message now entlich away ????? This interferes with the times! :-(
    1 of the crash took place after the Schließun (the second time).
    1 Hello, | crashed constantly on my Firefox 4.0 Beta 7th I have previously tried it with other versions but also the crashes. It is annoying my favorite browser but the constant crashes. | I was on the computerized images of bSeite
    1 you can get the button back to the old home place? (left). | printer I have to add every once and again.
    One oh that's not a good way to start

Comment 26

[John J. Barton]

I hit this when I run chromebug1.7, exit, then run the same profile without -chromebug on the command line, the exit.

http://crash-stats.mozilla.com/report/index/e31a856d-f5db-4186-bf3a-c232e2110112

Note that Extensions tab is incorrect on that report.  Says none, but that is false.

I guess that the comment:
One oh that's not a good way to start
was when I first began to use 4.0b9pre trunk, its the only time I remember this hitting when I was not exiting.

I do not have C++ support on this machine.

Comment 27

[Ben Turner (not reading bugmail, use the needinfo flag!)]

http://hg.mozilla.org/tracemonkey/rev/f2da7d8646f6

Comment 28

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/f2da7d8646f6