629961 - crash [@ nsDOMClassInfo::PreCreate], [@ XUL@0x62fba6 ]

Status: RESOLVED WORKSFORME

(Core :: XPConnect, defect)

Description

[David Humphrey (:humph)]

This bug was filed from the Socorro interface and is 
report bp-cb0bf4f8-4e83-4d37-9641-8bab72110129 .
============================================================= 

The last few days, working on our audio/webgl demo, I've been crashing on shutdown quite a bit.  It doesn't happen every time, and it doesn't always produce a useful stack (they might not all be the same thing).

Comment 1

[Andreas Gal :gal]

Setting b? to get this triaged.

Comment 2

[Andreas Gal :gal]

This looks like a shutdown problem. The context might have been destroyed already. Hard to tell from here. David, can you point me to the test case?

Comment 3

[David Humphrey (:humph)]

I'm having a lot of trouble reproducing this, but here is another one that just happened (not as helpful as before):

https://crash-stats.mozilla.com/report/index/bp-32583cf2-9003-482f-ab18-9a8c12110130

This demo is due tomorrow, so I'll have a version you can test with soon, but can't stop working on it now to get you one.

Comment 4

[David Humphrey (:humph)]

https://crash-stats.mozilla.com/report/index/bp-c30434c5-72f9-4f9a-8840-6a8142110130

Comment 5

[Andreas Gal :gal]

We can't block on this until we get a test case and STR.

Comment 6

[David Humphrey (:humph)]

I've had it happen 5 times over the weekend to me, and I still have no idea how to tell you STR, except that I'll mail you a link to this demo and you can try it there (we finished an hour ago)--you run it, sometimes on close, it crashes.  We are doing something with memory that doesn't agree with the browser.  This demo is likely a useful stress test of things anyway, so good for you to see it, Andreas.

Comment 7

[Andreas Gal :gal]

Yeah, that sounds good. Just send me the link and we will cycle through it a couple times.

Comment 8

[Andreas Gal :gal]

We should try to find out whats up here before we ship, if possible. We might not have to fix it before we ship.

Comment 9

[Andreas Gal :gal]

QA help would be appreciated reproducing this. If you need the test case, please contact me or David.

Comment 10

[Boris Zbarsky [:bzbarsky]]

That first stack seems pretty clear.  

   424 inline
   425 nsQueryInterface
   426 do_QueryWrapper(JSContext *cx, JSObject *obj)
   427 {
   428   nsISupports *native =
   429     nsDOMClassInfo::XPConnect()->GetNativeOfWrapper(cx, obj);
   430   return nsQueryInterface(native);
   431 }

nsDOMClassInfo::XPConnect() is null at this point in shutdown, presumably, given the crash address.

Comment 11

[Andreas Gal :gal]

I can make a patch based on that theory.

Comment 12

[Boris Zbarsky [:bzbarsky]]

An interesting question, though, is _why_ we're trying to js-wrap stuff that late in shutdown.  Are we leaking here?  humph's comments in #audio suggested we might be.

Comment 13

[Andreas Gal :gal]

Yeah an observer leak of some kind seems likely.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Fwiw, I'd kill for JS stacks in soccorro.  Probably not people, but a pigeon, say....  Does Ted take burnt offerings?  ;)

Comment 15

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Once we have OOP crashdumping, we can get there.

Comment 16

[Andreas Gal :gal]

Attachment: patch

Comment 17

[Andreas Gal :gal]

I think this is the right choke point. bz? This doesn't fix the leak of course.

Comment 18

[Andreas Gal :gal]

jst says that peter should take a look, too.

mrbkap says we don't always end up in nsDOMClassInfo::PreCreate, so we might want to add this to all PreCreate hooks.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 509285 [details] [diff] [review]
patch

Yeah, add to the other hooks and looks good.

Comment 20

[Peter Van der Beken [:peterv]]

Not all hooks would need this, windows and documents (so also nodes that still have a wrapper) keep nsLayoutStatics alive past XPCOM shutdown notifications. That was done to prevent these types of crashes. The scary part is that nsDOMScriptObjectFactory::Observe seems to circumvent it though.

Comment 21

[Andreas Gal :gal]

peterv, what should we do here?

Comment 22

[Andreas Gal :gal]

jst suggested peterv should take this since he knows what to do here

Comment 23

[Mike Beltzner [:beltzner, not reading bugmail]]

** PRODUCT DRIVERS PLEASE NOTE **

This bug is one of 7 automatically changed from blocking2.0:final+ to blocking2.0:.x during the endgame of Firefox 4 for the following reasons:

 - it was marked as a soft blocking issue without a requirement for beta coverage

Comment 24

[Ioana (away)]

According to Socorro, this bug is no longer reproducible:
https://crash-stats.mozilla.com/query/?product=Firefox&version=ALL%3AALL&range_value=4&range_unit=weeks&date=01%2F16%2F2014+14%3A00%3A00&query_search=signature&query_type=contains&query=XUL%400x62fba6&reason=&release_channels=&build_id=&process_type=any&hang_type=any
https://crash-stats.mozilla.com/query/?product=Firefox&version=ALL%3AALL&range_value=4&range_unit=weeks&date=01%2F16%2F2014+14%3A00%3A00&query_search=signature&query_type=contains&query=nsDOMClassInfo%3A%3APreCreate&reason=&release_channels=&build_id=&process_type=any&hang_type=any