"Assertion failure: thing->compartment() == gcmarker->context->runtime->gcCurrentCompartment"

RESOLVED FIXED

Status

()

Core
XPConnect
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: mrbkap)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Trunk
x86_64
Mac OS X
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse] fixed-in-tracemonkey)

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 528761 [details]
testcase (must be local; slow; crashes Firefox when loaded)

Assertion failure: thing->compartment() == gcmarker->context->runtime->gcCurrentCompartment, at js/src/jsgcmark.cpp:186
(Reporter)

Comment 1

6 years ago
Created attachment 528762 [details]
stack trace
(Reporter)

Comment 2

6 years ago
Why do I have to trigger GC using lots of strings in order to make this bug show up?  Usually, I can use my extension with fuzzPrivGC().
Thanks, Jesse. I just realized why you can't trigger a GC. The assertion you're getting only fires during a per-compartment GC. Shell functions like gc() (and presumably your extension) always cause a full GC.

I now realize that we really need a way to initiate a per-compartment GC artificially. I'll fold this functionality into bug 650978, but I imagine your extension will need to be updated as well. Would it be enough to add an arguments to JS_GC specifying which kind of GC to perform?
(Reporter)

Comment 4

6 years ago
> Would it be enough to add an
> arguments to JS_GC specifying which kind of GC to perform?

How would I tell it which compartment to do the GC in? Pass in a window object?
(Assignee)

Comment 5

6 years ago
It seems like the easiest way to implement the API would be to do a compartmental GC for the currently-running compartment.
(Reporter)

Comment 6

6 years ago
So the function would have to be exposed directly to web pages? I guess that's ok.
I looked into this, and it seems like it might be a compartment mismatch of some sort. There's an object of class NativePropertyHolder, whose slot zero points to an object of class Proxy that's in a different compartment. Blake wants to take a look at it.
Assignee: nobody → mrbkap
(Assignee)

Comment 8

6 years ago
Created attachment 529241 [details] [diff] [review]
Proposed fix

We talked about doing this a while ago. The patch was pretty simple in the end. Things seem to continue to work. Basically, we can hold a pointer to the WrappedNative* instead of its associated JSObject. Note the long comment about ownership added in the patch.
Attachment #529241 - Flags: review?(gal)
assuming the worst (sg:critical?) due to GC and pointers being involved.
Whiteboard: [sg:critical?]
Duplicate of this bug: 654930
Blake, do you think it would be safe to open this bug up? As we discussed, the GC will trace through exactly the same set of objects before and after the patch. So it seems safe to me.
I modified the per-compartment trigger mechanism in order to only perform per-compartment GCs. I saw this assert immediately on techcrunch.
With this patch, the assert seems to be gone.

Updated

6 years ago
Attachment #529241 - Flags: review?(gal) → review+
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/tracemonkey/rev/6b033d30337a
Group: core-security
Whiteboard: [sg:critical?] → [sg:nse] fixed-in-tracemonkey
Duplicate of this bug: 653330
Another set of steps to reproduce:
 1. load http://www.tnr.com/article/the-vital-center/87976/obama-democrats-republicans-pew-political-typology
 2. right click on the link "Pew Research Center publishes"
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/6b033d30337a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.