Cycle collector crash on shutdown with setUserData

RESOLVED FIXED in mozilla1.9beta1

Status

()

P1
critical
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: jruderman, Assigned: peterv)

Tracking

(Depends on: 1 bug, {crash, regression, testcase})

Trunk
mozilla1.9beta1
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
wanted1.8.1.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(1 attachment)

(Reporter)

Description

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

I'm guessing this is a regression from bug 401687.
Flags: blocking1.9?
(Reporter)

Comment 1

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

(Assignee)

Updated

11 years ago
Assignee: nobody → peterv
OS: Mac OS X → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9 M9
(Assignee)

Comment 2

11 years ago
Created attachment 287298 [details] [diff] [review]
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.
Attachment #287298 - Flags: superreview?(jst)
Attachment #287298 - Flags: review?(jonas)
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
Attachment #287298 - Flags: superreview?(jst)
Attachment #287298 - Flags: superreview+
Attachment #287298 - Flags: review?(jonas)
Attachment #287298 - Flags: review+
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.
Flags: blocking1.9? → blocking1.9+
Checked in. Leaving for peter to mark as fixed.

Comment 5

11 years ago
Bug# is wrong in/on bonsai.
I filed bug 402490 on my mistake (didn't know we had process for that, but apparently we do)
(Assignee)

Comment 7

11 years ago
Thanks for the reviews/checkin.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 8

11 years ago
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).
Depends on: 403358
Flags: in-testsuite?

Comment 9

11 years ago
This testcase now crashes on shutdown just like bug 403145
Group: security
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?
Depends on: 403145
Whiteboard: [sg:critical?]

Comment 11

11 years ago
That it now shows a double free on shutdown as in bug 403145 which is a different bug.
(Reporter)

Comment 12

11 years ago
The cycle collector was added after the 1.8 branch, so this bug doesn't affect the 1.8 branch.
Group: security
Flags: wanted1.8.1.x-
(Reporter)

Comment 13

11 years ago
I checked in the testcase for bug 324871 as content/base/crashtests/324871-1.html.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.