39321 - JS GC doesn't auto-suspend requests nested in multiple contexts on the same thread

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Brendan Eich [:brendan]]

The JS GC knows enough to exclude its calling context's request, by checking 
cx->requestDepth and if set, temporarily decrementing rt->requestCount.  But 
what if there are other JSContexts in use (exclusive, of course) by the same 
thread, and some of them are in active requests, executing under the call to the 
GC in nested activations?

One easy solution: scan rt->contextList for other contexts with the same 
cx->thread id as the one passed to js_GC, and for each that has cx->requestDepth 
non-zero, temporarily decrement rt->requestCount.

/be

Comment 1

[Brendan Eich [:brendan]]

Adding alecf.

Once I fix this, I should fix the bug where JS code entered without benefit of 
JS_BeginRequest and JS_EndRequest surrounding maximal non-blocking sequences of 
JS API calls, on any thread other than the UI/DOM/Layout "main" thread, is not 
GC-safe at present.  To be safe, we need to bracket such sequences with JS_Begin 
and EndRequest.

We don't need to bracket JS uses in the main thread with requests, because of 
this peculiarity: due to thread-unsafe finalizers that must run on the main 
thread, the JS GC can run only on that thread (alternative is to proxy all 
finalizers to the right thread, but that's wrong: besides being slow, it could 
make for deadlocks, and leaks in the event of proxy memory shortfalls -- JS 
finalizers are void for a reason, they must infallibly return all memory).  So 
JS run on the main thread need never fear being preempted in a GC-unsafe place 
by GC on another thread -- GC runs only on the main thread, and at safe "GC 
preemption points".

But those other threads running JS -- they need the request model, or the main 
thread might race badly through the GC with their JS compilation and API usage.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 3

[Brendan Eich [:brendan]]

Fix checked in.  Now to fix the non-main-thread JS_* uses to be within requests. 
Separate bug for that coming soon.

/be