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

RESOLVED FIXED

Status

()

Core
DOM
--
critical
RESOLVED FIXED
12 years ago
4 years ago

People

(Reporter: Stellan Klebom, Assigned: dbaron)

Tracking

({crash, fixed1.8.1})

Trunk
crash, fixed1.8.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch], crash signature)

Attachments

(4 attachments)

(Reporter)

Description

12 years ago
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

12 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

12 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.
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

12 years ago
http://downloads.mozdev.org/ieview/ieview.xpi
http://downloads.mozdev.org/launchy/launchy.xpi
http://ftp.mozilla.org/pub/mozilla.org/extensions/greasemonkey/greasemonkey-0.6.4-fx.xpi
http://adblock.ethereal.net/SessionSaver/sessionsaver-02-dev.xpi
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
Created attachment 210289 [details]
Stack trace plus some debugging info

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

12 years ago
Summary: Firefox 1.6a1 crash on exit (extension related) → Firefox 1.6a1 crash [@ ClassifyWrapper] on exit (extension related)

Comment 7

12 years ago
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?
Created attachment 210442 [details] [diff] [review]
Some debugging stuff

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
(Assignee)

Comment 11

12 years ago
We probably need to call elm->Disconnect() at step (3), although I'm not crazy about calling destructors then...
(Assignee)

Updated

12 years ago
Blocks: 241518
(Assignee)

Updated

12 years ago
Assignee: general → dbaron
Blocks: 325738
(Assignee)

Comment 12

12 years ago
Created attachment 211521 [details] [diff] [review]
possibility #1
(Assignee)

Comment 13

12 years ago
Created attachment 211522 [details] [diff] [review]
possibility #2
(Assignee)

Updated

12 years ago
Attachment #211522 - Flags: superreview?(jst)
Attachment #211522 - Flags: review?(bzbarsky)
(Assignee)

Updated

12 years ago
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+
(Assignee)

Comment 16

12 years ago
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.

Comment 18

12 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

12 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

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Flags: blocking1.9a1?
Resolution: --- → FIXED

Comment 20

12 years ago
*** 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.
(Assignee)

Updated

12 years ago
Blocks: 336791
(Assignee)

Comment 22

12 years ago
Fixed on MOZILLA_1_8_BRANCH by checkin of bug 336791.
Keywords: fixed1.8.1
(Assignee)

Updated

11 years ago
Depends on: 350930
Crash Signature: [@ ClassifyWrapper]
You need to log in before you can comment on or make changes to this bug.