334177 - topcrash (not at shutdown) [@ PL_DHashTableRawRemove] called from nsGenericElement::~nsGenericElement

Status: VERIFIED FIXED

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

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Bug 237736 has tracked crashes in PL_DHashTableRawRemove called from ~nsGenericElement.  It was originally filed on a shutdown crash, but has come to track both that problem and the topcrash we've been seeing.  I'm filing this bug as a separate bug to analyze the topcrash.

I did a detailed analysis of one of the stacks in talkback with this signature, figured out on which instruction the crash was happening (the crash is on the line:
        MARK_ENTRY_REMOVED(entry);
assigning 0x1 into entry->keyHash.

And I've finally figured out *why* this is happening -- the clearEntry callback used for sEventListenerManagersHash can mutate (shrink, most likely) the table!

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: detailed analysis of TB17589254, a Linux Fx 1.5.0.1 incident

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In particular, what I suspect is happening is that we end up with a stack something like (this entire stack is written by hand):

ChangeTable
PL_DHashTableOperate
nsGenericElement::~nsGenericElement
...
<removal of a C++-implemented event listener>
nsEventListenerManager::ReleaseListeners
nsEventListenerManager::RemoveAllListeners
nsEventListenerManager::~nsEventListenerManager
nsEventListenerManager::Release
nsCOMPtr_base::~nsCOMPtr_base
nsCOMPtr<nsIEventListenerManager>::~nsCOMPtr<nsIEventListenerManager>
EventListenerManagerMapEntry::~EventListenerManagerMapEntry
EventListenerManagerClearEntry
PL_DHashTableRawRemove   <== crash here on un-wind
PL_DHashTableOperate
nsGenericElement::~nsGenericElement

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

FWIW, I filed bug 334180 on making pldhash assert about this type of problem.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: trunk patch

The branch patch will look a bit different thanks to bug 315901.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch for 1.8 branch

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 218870 [details] [diff] [review]
trunk patch

r+sr=jst

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk and MOZILLA_1_8_BRANCH.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 218872 [details] [diff] [review]
patch for 1.8 branch

aproved for 1.8.0 branch, a=dveditz for drivers

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to MOZILLA_1_8_0_BRANCH.

Comment 10

[Tracy Walker [:tracy]]

no longer appearing on topcrash reports for branch. However, PL_DHashTableRawRemove still appears on trunk topcrash reports. But that may be related to bug 234169. Can't tell right now as incedent query is broken.