Closed Bug 393038 Opened 17 years ago Closed 17 years ago

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

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED DUPLICATE of bug 404879

People

(Reporter: jorendorff, Unassigned)

References

Details

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.
Flags: in-testsuite-
The new plan is to do this after lifting the SpiderMonkey request model into Tamarin.
Summary: ActionMonkey: js_GC should scan other threads' stacks → ActionMonkey: MMgc::GC::Collect() should scan other threads' stacks
Assignee: general → nobody
Component: JavaScript Engine → Garbage Collection (mmGC)
Flags: in-testsuite-
Product: Core → Tamarin
QA Contact: general → gc
Version: Other Branch → unspecified
Depends on: 395962
Blocks: 394297
No longer blocks: 393023
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).
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.