402404 - Cycle collector crash on shutdown with setUserData

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Loading attachment 223709 [details] (the testcase for bug 324871) makes Firefox crash on shutdown.

I'm guessing this is a regression from bug 401687.

Comment 1

[Jesse Ruderman]

Just before the crash, I see:

###!!! ASSERTION: Must be tracaeble: 'JSVAL_IS_TRACEABLE(mJSVal)', file /Users/jruderman/trunk/mozilla/js/src/xpconnect/src/xpcvariant.cpp, line 66

###!!! ASSERTION: Must be linked: 'mSelfp', file /Users/jruderman/trunk/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp, line 1404

Comment 2

[Peter Van der Beken [:peterv]]

Attachment: v1

After unlinking an XPCTraceableVariant we remove it from the set of XPConnect roots and set its mJSVal to JSVAL_NULL. We must not remove it twice from the set of XPConnect roots (that causes a crash), so in the destructor we need to check if its non-null before trying to remove it. JSVAL_IS_TRACEABLE is defined as |(JSVAL_IS_GCTHING(v) && !JSVAL_IS_NULL(v))|, so the assertion should use JSVAL_IS_GCTHING and we need to check for !JSVAL_IS_NULL before removing it from the root set. Again, whoever gets to this first please r/sr.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm gonna take the liberty to land this even though it isn't formally approved yet. Feel free to back me out if you disagree.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in. Leaving for peter to mark as fixed.

Comment 5

[pal-moz]

Bug# is wrong in/on bonsai.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I filed bug 402490 on my mistake (didn't know we had process for that, but apparently we do)

Comment 7

[Peter Van der Beken [:peterv]]

Thanks for the reviews/checkin.

Comment 8

[Jeff Walden [:Waldo]]

I filed bug 403358 on getting Mochitests for the DOM userdata functionality, since apparently we have none at the moment (and thus existing tests didn't catch this).

Comment 9

[Bob Clary [:bc] (inactive)]

This testcase now crashes on shutdown just like bug 403145

Comment 10

[Daniel Veditz [:dveditz]]

what does comment 9 mean? That this testcase now exposes a different bug (don't need to do anything more here) or that it's not fixed?

Comment 11

[Bob Clary [:bc] (inactive)]

That it now shows a double free on shutdown as in bug 403145 which is a different bug.

Comment 12

[Jesse Ruderman]

The cycle collector was added after the 1.8 branch, so this bug doesn't affect the 1.8 branch.

Comment 13

[Jesse Ruderman]

I checked in the testcase for bug 324871 as content/base/crashtests/324871-1.html.