Jason Orendorff [:jorendorff] Reporter
	Description • 15 years ago

If a thread calls JS_GC outside of any request, and GC is happening in another thread, then JS_GC does not wait for GC to finish before returning.

I'm not sure this is a bug, but it does seem a little counterintuitive. At the least, there's a comment in the code that's not entirely correct:

        /*
         * If the GC runs on another thread, temporarily suspend all requests
         * running on the current thread and wait until the GC is done.
         */
        if (rt->gcThread != cx->thread) {
            requestDebit = js_CountThreadRequests(cx);
            ...
            if (requestDebit != 0) {
                ...code to suspend all requests...
                ...code to wait until GC is done...
                ...code to resume requests...
            }
        }

timeless pointed out that the next time the caller enters a request, it will wait for GC to finish anyway. And we do otherwise allow threads outside of requests to race with GC. So perhaps this is a feature and just the comment and the API doc for JS_GC need to be changed.

Please chime in. This is super easy to fix either way.

	Igor Bukanov Assignee
	Comment 1 • 14 years ago

(In reply to comment #0)
> I'm not sure this is a bug, 

This is a bug. Although such GC will wait for all requests to finish, it will set gcThread to point to itself. This at least breaks the assumption about gcThread in the title-related code.

	Igor Bukanov Assignee
	Comment 2 • 14 years ago

Note that this bug does not affect firefox even with addons installed as the GC callback ensures that the GC is only called from the main thread.

	Igor Bukanov Assignee
	Comment 3 • 14 years ago

The patch makes LetOtherGCToFinish to wait even if js_GC is called outside a request.

	Jason Orendorff [:jorendorff] Reporter
	Comment 5 • 14 years ago

Comment on attachment 445524 [details] [diff] [review]
fix v1

>+     * When the GC is called outside the request and requestDebit is zero we
>+     * still wait here even if the GC on another thread is waiting in
>+     * BeginGCSession for requests from all threads to finish and gcRunning
>+     * is false. It ensures that js_GC is not returned until the GC cycle is
>+     * finished on another thread.

How about:

    /*
     * Wait for GC to finish on the other thread, even if requestDebit is 0
     * and even if GC has not started yet because the gcThread is waiting in
     * BeginGCSession. This ensures that js_GC never returns without a full
     * GC cycle happening.
     */

	Brendan Eich [:brendan]
	Comment 6 • 14 years ago

s/Let/Allow/ in LetOtherGCToFinish -- or else s/To// and keep the Let.

/be

	Igor Bukanov Assignee
	Comment 7 • 14 years ago

http://hg.mozilla.org/tracemonkey/rev/baefd422dd60 - now it is LetOtherGCFinish

	Robert Sayre
	Comment 8 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/baefd422dd60