393038 - ActionMonkey: MMgc::GC::Collect() should scan other threads' stacks

Status: VERIFIED DUPLICATE of bug 404879

(Tamarin Graveyard :: Garbage Collection (mmGC), defect)

Description

[Jason Orendorff [:jorendorff]]

With JS_THREADSAFE, GC happens only when each thread in the process is either (a) not in any request, in which case we don't care about it; or (b) in a request, but safely parked, tires curbed, waiting for GC to finish.

However, MMgc only scans the calling thread's stack.  This is a pretty huge thread-safety hole.  I don't think it affects Gecko, but it will affect embedders using SpiderMonkey for server-side scripting.

The fix is not so hard: in jsgc.cpp:js_GC, before calling JS_AWAIT_GC_DONE, create a GCRoot for the stack of the calling thread. (This should be done using the MMGC_GET_STACK_EXTENTS macro, defined in MMgc/GC.h.)

(Making MMgc fully thread-safe will need a separate bug.)

It would be great to have a test case for this.

Comment 1

[Jason Orendorff [:jorendorff]]

The new plan is to do this after lifting the SpiderMonkey request model into Tamarin.

Comment 2

[Jason Orendorff [:jorendorff]]

This is covered by bug 395962 (for places where MMgc needs to root the stack) and bug 404879 (for extra places where SpiderMonkey needs to root the stack).  The latter in particular discusses stack-rooting in JS_SuspendRequest, which unfortunately is rather costly.

As for test cases, I've suffered through plenty of those.  :)  Bug 404879 contains tests that exercise these stack roots; they found several bugs in my patch (and led me to file bug 408065 against SpiderMonkey).