Talkback is showing one of the Firefox2 topcrashes is in PL_DHashTableOperate (accounting for most of the PL_DHashTableOperate crashes), operating on nsDOMClassInfo's sExternallyReferencedTable with the stack that is presumably: PL_DHashTableOperate nsDOMClassInfo::UnsetExternallyReferenced nsImageLoadingContent::UnpreserveLoadHandlers nsHTMLImageElement::~nsHTMLImageElement

This is an example Linux incident, Windows incident, and Mac incident. The Windows incident is typical of many, although some have one more frame worth of information (DOMGCCallback).

Currently #10 FF2 crasher. Could it be a problem that there is a QI and Addred/Release happening while deleting nsHTMLImageElement?

Let's try to get some traction on this bug if we want to fix a topcrasher for the next release. Anyone want to take a shot at this one?

Could we just add an if(.ops) check. This way if something is keeping an htmlimage alive after nsDOMClassInfo::ShutDown we don't crash.

But nsDOMClassInfo::ShutDown says: if (sExternallyReferencedTable.ops && sExternallyReferencedTable.entryCount == 0) { Did you see indications in talkback comments that this was a shutdown crash?

(In reply to comment #5) > > Did you see indications in talkback comments that this was a shutdown crash? > Some of comments are like: "I closed the browser", "Restarting Firefox after updating Add-ons." "exited Firefox and pressed the OK button to clean up cookies, temp files etc."

(In reply to comment #5) > But nsDOMClassInfo::ShutDown says: > > if (sExternallyReferencedTable.ops && > sExternallyReferencedTable.entryCount == 0) { > Ah, entryCount == 0. Is it possible the htmlimage isn't always added to sExternallyReferencedTable, so that entryCount is 0, but somehow there is an htmlimage alive...

Comment on attachment 250154 [details] [diff] [review] possible patch There are some known cases where these calls are unbalanced, so let's try this. r+sr=dbaron But could you add a pair of assertions -- one that ops is non-null, and the second that the entryCount actually decreases across the remove?

Checked in to trunk. Waiting few days to see whether this helps and then asking approval for 1.8.1.2.

Comment on attachment 250187 [details] [diff] [review] +assertions >+ NS_ASSERTION(sExternallyReferencedTable.ops, >+ "Unexpected null sExternallyReferencedTable.ops"); I'd make the text be "unbalanced call". >+ NS_ASSERTION(count != sExternallyReferencedTable.entryCount, This could be count == sExternallyReferencedTable.entryCount + 1 >+ "DOMGCParticipant wasn't removed from sExternallyReferencedTable."); And likewise about the text.

This was checked in to trunk and applies cleanly to 1.8 branch. Haven't seen any trunk crashes since the landing, but those have been quite rare anyway, so waiting still few days before asking approval.

Comment on attachment 250468 [details] [diff] [review] checked in hmm, though, perhaps it is better to have testing also on branch. The patch shouldn't cause any harm in any case, since it is just a null check + assertions.

Comment on attachment 250468 [details] [diff] [review] checked in Approved for 1.8 branch, a=jay for drivers. Let's get this landed now and keep an eye on crash data for any regressions and to make sure this is fixed.

Checked in to 1.8.

Marking fixed, but will look at the crash data and reopen if needed.

v.fixed on Trunk and 1.8 branch based on Talkback data (or lack thereof). I only see one incident from 12/17 branch nightly (tons with 2.0.0.1 from 12/4), and only a few on the Trunk from a while back. None since.