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

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Jesse Ruderman]

Attachment: testcase (must be local; slow; crashes Firefox when loaded)

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

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Jesse Ruderman]

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().

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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?

Comment 4

[Jesse Ruderman]

> 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?

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

It seems like the easiest way to implement the API would be to do a compartmental GC for the currently-running compartment.

Comment 6

[Jesse Ruderman]

So the function would have to be exposed directly to web pages? I guess that's ok.

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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.

Comment 9

[Daniel Veditz [:dveditz]]

assuming the worst (sg:critical?) due to GC and pointers being involved.

Comment 11

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 12

[Gregor Wagner [:gwagner]]

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.

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/6b033d30337a

Comment 15

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

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"

Comment 16

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/6b033d30337a