Last Comment Bug 661927 - API to force a precise GC (return to event loop, GC with no JS stack, then call a callback)
: API to force a precise GC (return to event loop, GC with no JS stack, then ca...
Status: RESOLVED FIXED
[sg:want]
: dev-doc-complete
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla7
Assigned To: Josh Matthews [:jdm]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 722771 669617 698151 1161491
Blocks: LifetimeTesting
  Show dependency treegraph
 
Reported: 2011-06-03 13:44 PDT by Jesse Ruderman
Modified: 2015-05-05 03:23 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running. (7.09 KB, patch)
2011-06-24 16:14 PDT, Josh Matthews [:jdm]
no flags Details | Diff | Splinter Review
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running. (7.12 KB, patch)
2011-06-24 16:20 PDT, Josh Matthews [:jdm]
no flags Details | Diff | Splinter Review
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running. (6.19 KB, patch)
2011-06-29 14:20 PDT, Josh Matthews [:jdm]
mrbkap: review+
Details | Diff | Splinter Review
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running. (6.37 KB, patch)
2011-06-30 08:25 PDT, Josh Matthews [:jdm]
mrbkap: review+
Details | Diff | Splinter Review
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running. (6.31 KB, patch)
2011-06-30 23:09 PDT, Josh Matthews [:jdm]
no flags Details | Diff | Splinter Review

Description User image Jesse Ruderman 2011-06-03 13:44:56 PDT
For quitWithLeakCheck (and perhaps other automated tests), I'd like to have a way to cause a GC to happen with no JS on the stack, so I don't have to worry about conservative GC false positives.
Comment 1 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-23 21:16:57 PDT
Do we want to run other things on the event loop here, or do we want to just remove what's entrained on the stack from consideration?
Comment 2 User image Josh Matthews [:jdm] 2011-06-24 08:07:36 PDT
Hmm, is the idea that you would just dispatch a runnable that would check if there's JS on the stack and redispatch itself if it couldn't gc yet?
Comment 3 User image Andrew McCreight [:mccr8] 2011-06-24 08:29:32 PDT
I'd think that the function would take a JS function as an argument.  When run, it would blow away the JS stack, and invoke the function argument, which is being used as a continuation.  A JS-exec, if you prefer.
Comment 4 User image Jesse Ruderman 2011-06-24 10:26:22 PDT
Comment 2 sounds right, given the possibility of nested event loops.

No synchronously "blowing away the stack", please :)
Comment 5 User image Josh Matthews [:jdm] 2011-06-24 10:55:35 PDT
Sounds like fun.
Comment 6 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-24 10:57:39 PDT
I would think for fuzzing that you'd want to be able to make GC happen "right now", or as close to right now as is possible.  That would mean that you wouldn't want to run extra things off the event loop, which comment 2 would do ...
Comment 7 User image Andrew McCreight [:mccr8] 2011-06-24 11:04:49 PDT
Clearly Javascript needs a general call-cc mechanism.
Comment 8 User image Jesse Ruderman 2011-06-24 11:47:09 PDT
I can already make GC happen "right now", and I use that for fuzzing. What I'm asking for here would be used for precise leak checks at the end of fuzzing, so it's ok if it's stuck on the end of the event queue.
Comment 9 User image Josh Matthews [:jdm] 2011-06-24 16:14:42 PDT
Created attachment 541852 [details] [diff] [review]
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running.
Comment 10 User image Josh Matthews [:jdm] 2011-06-24 16:20:59 PDT
Created attachment 541856 [details] [diff] [review]
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running.
Comment 11 User image Olli Pettay [:smaug] (review queue closed until backlog cleared) 2011-06-26 13:55:28 PDT
How does the patch guarantee that there is no JS running?
Comment 12 User image Josh Matthews [:jdm] 2011-06-26 19:51:39 PDT
I assumed that simply checking JS_IsRunning on the desired context would be enough. Should I instead acquire the xpconnect JS runtime, then iterate over all its contexts and do the same check?
Comment 13 User image Olli Pettay [:smaug] (review queue closed until backlog cleared) 2011-06-27 05:06:26 PDT
My mistake. I missed the JS_IsRunning in the ::Run().
(I don't know what JS_IsRunning does)

Another question, couldn't you have some idl interface to implement the callback?
Comment 14 User image Josh Matthews [:jdm] 2011-06-27 06:48:23 PDT
I could. I don't really see why that's desirable, however.
Comment 15 User image Olli Pettay [:smaug] (review queue closed until backlog cleared) 2011-06-27 07:18:44 PDT
That way the callback would automatically support both function() {} and
{ foo: function() {} }. And using JS API is quite error prone.
Comment 16 User image Blake Kaplan (:mrbkap) 2011-06-29 12:14:28 PDT
Comment on attachment 541856 [details] [diff] [review]
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running.

I think I agree with smaug here. If you take an XPCOM interface, you won't have to deal with rooting or context pushing or anything else like that. I'm not sure if the JS_IsRunning check is sufficient. It's possible that other JS code could be running on a different context. On the other hand, I'm not sure if we have an easy way of finding that out...
Comment 17 User image Josh Matthews [:jdm] 2011-06-29 12:36:35 PDT
> I'm not sure if the JS_IsRunning check is sufficient. It's possible that other
> JS code could be running on a different context. On the other hand, I'm not
> sure if we have an easy way of finding that out...

It's possible to grab the runtime service, get the JSRuntime and then use a context iterator to check every context therein. Would that be more suitable?
Comment 18 User image Josh Matthews [:jdm] 2011-06-29 14:20:45 PDT
Created attachment 542941 [details] [diff] [review]
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running.
Comment 19 User image Blake Kaplan (:mrbkap) 2011-06-29 14:29:33 PDT
Comment on attachment 542941 [details] [diff] [review]
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running.

Yeah, this is much cleaner.

One nit: your test addition to the makefile should use tabs instead of spaces for spacing. r=mrbkap with that.
Comment 21 User image Josh Matthews [:jdm] 2011-06-29 16:29:21 PDT
Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/5773d613df94 due to persistent windows build failures.

xpccomponents.obj : error LNK2019: unresolved external symbol "struct JSContext * __cdecl js_ContextIterator(struct JSRuntime *,int,struct JSContext * *)" (?js_ContextIterator@@YAPAUJSContext@@PAUJSRuntime@@HPAPAU1@@Z) referenced in function "public: virtual unsigned int __stdcall PreciseGCRunnable::Run(void)" (?Run@PreciseGCRunnable@@UAGIXZ)
NEXT ERROR xul.dll : fatal error LNK1120: 1 unresolved externals
Comment 22 User image Josh Matthews [:jdm] 2011-06-30 08:25:57 PDT
Created attachment 543136 [details] [diff] [review]
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running.

Third time's the charm.
Comment 23 User image Blake Kaplan (:mrbkap) 2011-06-30 17:19:07 PDT
Comment on attachment 543136 [details] [diff] [review]
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running.

You don't want the request around the JS_GC call. Otherwise, we'll still scan the stack. Other than that, this looks good.
Comment 24 User image Josh Matthews [:jdm] 2011-06-30 23:09:52 PDT
Created attachment 543358 [details] [diff] [review]
Add Cu.schedulePreciseGC to allow for a GC to run with no JS code running.

Patch for landing.
Comment 25 User image Dão Gottwald [:dao] 2011-07-02 00:49:33 PDT
http://hg.mozilla.org/mozilla-central/rev/7898841a922a

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