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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: klebom, Assigned: dbaron)
References
Details
(Keywords: crash, fixed1.8.1, Whiteboard: [patch])
Crash Data
Attachments
(4 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
bzbarsky
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
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?
Comment 2•19 years ago
|
||
Could you install with talkback and get a talkback ID for the crash? http://kb.mozillazine.org/Talkback
Severity: normal → critical
Keywords: crash
Comment 3•19 years ago
|
||
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.
Comment 4•19 years ago
|
||
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.
Comment 5•19 years ago
|
||
Updated•19 years ago
|
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
Comment 6•19 years ago
|
||
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?
Updated•19 years ago
|
Summary: Firefox 1.6a1 crash on exit (extension related) → Firefox 1.6a1 crash [@ ClassifyWrapper] on exit (extension related)
Comment 7•19 years ago
|
||
Bug 323371 is also a crash [@ ClassifyWrapper] and also involves Greasemonkey.
Comment 8•19 years ago
|
||
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?
Comment 9•19 years ago
|
||
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.
Comment 10•19 years ago
|
||
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
Assignee | ||
Comment 11•19 years ago
|
||
We probably need to call elm->Disconnect() at step (3), although I'm not crazy about calling destructors then...
Assignee | ||
Updated•19 years ago
|
Assignee: general → dbaron
Assignee | ||
Comment 12•19 years ago
|
||
Assignee | ||
Comment 13•19 years ago
|
||
Assignee | ||
Updated•19 years ago
|
Attachment #211522 -
Flags: superreview?(jst)
Attachment #211522 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•19 years ago
|
Whiteboard: [patch]
Comment 14•19 years ago
|
||
Comment on attachment 211522 [details] [diff] [review]
possibility #2
Makes sense, I guess.
Attachment #211522 -
Flags: review?(bzbarsky) → review+
Comment 15•19 years ago
|
||
Comment on attachment 211522 [details] [diff] [review]
possibility #2
sr=jst
Attachment #211522 -
Flags: superreview?(jst) → superreview+
Assignee | ||
Comment 16•19 years ago
|
||
Checked in to trunk. Let's see how this does...
Comment 17•19 years ago
|
||
(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.
Comment 18•19 years ago
|
||
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
Reporter | ||
Comment 19•19 years ago
|
||
(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!
Updated•19 years ago
|
Status: NEW → RESOLVED
Closed: 19 years ago
Flags: blocking1.9a1?
Resolution: --- → FIXED
Comment 20•19 years ago
|
||
*** Bug 325738 has been marked as a duplicate of this bug. ***
No longer blocks: 325738
Comment 21•19 years ago
|
||
I filed Bug 327859 for another crash on exit case.
Assignee | ||
Comment 22•18 years ago
|
||
Fixed on MOZILLA_1_8_BRANCH by checkin of bug 336791.
Keywords: fixed1.8.1
Updated•13 years ago
|
Crash Signature: [@ ClassifyWrapper]
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•