Last Comment Bug 653309 - "Assertion failure: thing->compartment() == gcmarker->context->runtime->gcCurrentCompartment"
: "Assertion failure: thing->compartment() == gcmarker->context->runtime->gcCur...
Status: RESOLVED FIXED
[sg:nse] fixed-in-tracemonkey
: assertion, testcase
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
:
Mentors:
: 653330 654930 (view as bug list)
Depends on:
Blocks: 326633 594645
  Show dependency treegraph
 
Reported: 2011-04-27 17:38 PDT by Jesse Ruderman
Modified: 2011-05-10 15:15 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (must be local; slow; crashes Firefox when loaded) (592 bytes, text/html)
2011-04-27 17:38 PDT, Jesse Ruderman
no flags Details
stack trace (5.00 KB, text/plain)
2011-04-27 17:39 PDT, Jesse Ruderman
no flags Details
Proposed fix (13.16 KB, patch)
2011-04-29 17:00 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
gal: review+
Details | Diff | Review

Description Jesse Ruderman 2011-04-27 17:38:09 PDT
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
Comment 1 Jesse Ruderman 2011-04-27 17:39:36 PDT
Created attachment 528762 [details]
stack trace
Comment 2 Jesse Ruderman 2011-04-27 17:40:20 PDT
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 (:billm) 2011-04-27 19:46:21 PDT
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 2011-04-28 14:41:17 PDT
> 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) (please use needinfo!) 2011-04-28 15:21:07 PDT
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 2011-04-28 16:13:20 PDT
So the function would have to be exposed directly to web pages? I guess that's ok.
Comment 7 Bill McCloskey (:billm) 2011-04-28 17:27:33 PDT
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) (please use needinfo!) 2011-04-29 17:00:03 PDT
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.
Comment 9 Daniel Veditz [:dveditz] 2011-05-03 17:11:56 PDT
assuming the worst (sg:critical?) due to GC and pointers being involved.
Comment 10 Bill McCloskey (:billm) 2011-05-05 10:08:59 PDT
*** Bug 654930 has been marked as a duplicate of this bug. ***
Comment 11 Bill McCloskey (:billm) 2011-05-05 10:11:51 PDT
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] 2011-05-05 12:45:34 PDT
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) (please use needinfo!) 2011-05-05 15:15:34 PDT
http://hg.mozilla.org/tracemonkey/rev/6b033d30337a
Comment 14 Bill McCloskey (:billm) 2011-05-05 15:31:24 PDT
*** Bug 653330 has been marked as a duplicate of this bug. ***
Comment 15 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-05-08 19:05:47 PDT
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) 2011-05-10 15:15:09 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/6b033d30337a

Note You need to log in before you can comment on or make changes to this bug.