Closed Bug 325279 Opened 19 years ago Closed 19 years ago

Firefox 1.6a1 crash [@ ClassifyWrapper] on exit (extension related)

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: klebom, Assigned: dbaron)

References

Details

(Keywords: crash, fixed1.8.1, Whiteboard: [patch])

Crash Data

Attachments

(4 files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060130 Firefox/1.6a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060130 Firefox/1.6a1 I have 51 Firefox extensions in use. I've noted crashes on exit for 1.6a1 nightlies for weeks now, and decided to find out if an extension was to blame... After a lot enabling/disabling extensions I found out having all four of IE View 1.2.7 Launchy 4.2.0 Greasmonkey 0.6.4 SessionSaver .2 d1 * nightly 30 enabled caused Firefox to crash on exit. Disabling anyone of them makes the problem go away. This with all other 47 either enabled or disabled. Reproducible: Always Steps to Reproduce: 1. Just install and enable all for extensions in details 2. quit firefox application Actual Results: crash
Any chance you could start pulling out pieces of those extensions to narrow down what exactly they're doing?
Could you install with talkback and get a talkback ID for the crash? http://kb.mozillazine.org/Talkback
Severity: normal → critical
Keywords: crash
Confirming on fresh trunk install with these four extensions installed. Talkback ID is TB14581295M. Note that Greasemonkey is marked incompatible with the trunk, the others are not.
Can someone post links to those extensions so that this can be easily tested? I'm willing to try debugging this, but not to spend half a day hunting down extensions.
Assignee: nobody → general
Status: UNCONFIRMED → NEW
Component: General → DOM
Ever confirmed: true
OS: Windows XP → All
Product: Firefox → Core
QA Contact: general → ian
Hardware: PC → All
Version: unspecified → Trunk
Attached file Stack trace plus some debugging info (deleted) —
Justin, thanks for the links! So it looks like we have a bogus entry in the wrapper table. I'm really not sure how that can happen. David, any ideas? Does it matter that we're reentering GC here?
Summary: Firefox 1.6a1 crash on exit (extension related) → Firefox 1.6a1 crash [@ ClassifyWrapper] on exit (extension related)
Bug 323371 is also a crash [@ ClassifyWrapper] and also involves Greasemonkey.
So when I crash, I see things looking like this: (gdb) p entry->participant $1 = (class nsIDOMGCParticipant *) 0x88e6a00 (gdb) x/wa *(void**) entry->participant 0x897e578: 0x10 So it's dead. During the run, I have logged: Created wrapped native [object HTMLAnchorElement @ 0x89d3778 (native @ 0x88e6a00)], flat JSObject is 0x8593c48 Created new XPCNativeWrapper 0x8593c28 for wrapped native [object HTMLAnchorElement @ 0x89d3778 (native @ 0x88e6a00)] which are the relevant JSObjects here. I added some logging to the relevant Finalize hooks, and I do see them both being called. In this case, the entry looks like: (gdb) p *entry $2 = {<PLDHashEntryHdr> = {keyHash = 369669479}, key = 0x89fee58, keyToWrapperFunc = 0xb7545dba <HolderToWrappedJS>, participant = 0x88e6a00, next = 0x0} so it looks like we're somehow failing to remove the preservation from an event listener... Note that earlier in the same run I saw: ###!!! ASSERTION: nsGenericElement's event listener manager hash not empty at shutdown!: 'sEventListenerManagersHash.entryCount == 0', file ../../../../mozilla/content/base/src/nsGenericElement.cpp, line 835 So it looks like we're leaking the event listener manager and/or the content node... probably because of something the extension(s) is/are doing. Then note that clearing the ELM hash at shutdown doesn't actually do proper clearing of the entries (on purpose), and I can see how we end up with the situation we have. So is there any way we can deal with this gracefully? Or just tell extensions to stop leaking? ;)
Blocks: 323371
Flags: blocking1.9a1?
Attached patch Some debugging stuff (deleted) — Splinter Review
Well, the first part is debugging. The second part is a necessary correctness fix, I think. I thought it might help here, but it didn't.
So to summarize what I _think_ happens (I can verify via more detailed logging if desired): 1) We start shutdown. 2) We shut down layout. At this point, the element in question is still owned by its JSObject, which has not been collected because the extension is holding on to it somehow (dunno how). 3) The assertion in question fires, and we finish the event listener manager table but don't actually call the holder destructors. As a result, there is now a permanent entry for our element in the wrapper hash. 4) Shutdown continues and at some point the extension gets a shutdown notification and releases stuff. 5) Garbage collection happens and the element is destroyed after its JSObject is garbage collected. 6) More garbage collection is triggered (via JSContext destruction) and we crash because our wrapper cache has an entry pointing to a deleted GCParticipant. So to fix we either need to change what happens in step 3 or somehow make the whole shutdown thing happier here, I think.
Depends on: 316414
We probably need to call elm->Disconnect() at step (3), although I'm not crazy about calling destructors then...
Blocks: 241518
Assignee: general → dbaron
Blocks: 325738
Attached patch possibility #1 (deleted) — Splinter Review
Attached patch possibility #2 (deleted) — Splinter Review
Attachment #211522 - Flags: superreview?(jst)
Attachment #211522 - Flags: review?(bzbarsky)
Whiteboard: [patch]
Comment on attachment 211522 [details] [diff] [review] possibility #2 Makes sense, I guess.
Attachment #211522 - Flags: review?(bzbarsky) → review+
Comment on attachment 211522 [details] [diff] [review] possibility #2 sr=jst
Attachment #211522 - Flags: superreview?(jst) → superreview+
Checked in to trunk. Let's see how this does...
(In reply to comment #16) > Checked in to trunk. Let's see how this does... > So far so good David. I used to 100% crash on Bug 325738 and I am unable to reproduce that now (no crash in 20 attempts) I've asked testers on the forum to reenable the extensions that were related to this and try to crash their build.
The crash on exit issues seem to have gone away for me too, with the most recent trunk daily: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060218 Firefox/1.6a1 ID:2006021806
(In reply to comment #16) > Checked in to trunk. Let's see how this does... As filer of this bug I can with great joy share the fact that build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060218 Firefox/1.6a1 ID:2006021806 no longer crashes on exit when all four of above extensions are enabled. Thanks Boris and David!
Status: NEW → RESOLVED
Closed: 19 years ago
Flags: blocking1.9a1?
Resolution: --- → FIXED
*** Bug 325738 has been marked as a duplicate of this bug. ***
No longer blocks: 325738
I filed Bug 327859 for another crash on exit case.
Blocks: 336791
Fixed on MOZILLA_1_8_BRANCH by checkin of bug 336791.
Keywords: fixed1.8.1
Depends on: 350930
Crash Signature: [@ ClassifyWrapper]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: