JS public API allows formally to enter requests from several contexts on the same thread. Although the code tries to deal with this (like GC scanning all JSContext on the current thread accounting for all entered requests), such practice is error prone and can trivially can lead to deadlocks. In particular, JS_SuspendRequest and JS_YieldRequest only suspend and yield the request for the passes cx instance, they do not consider other contexts. Precisely for these reasons AFAICS FF code based already ensures that only one cx is active. To address this I suggest to explicitly require that only one JSContext instance can enter into the request on a given thread. The embeddings that need to deal with several contexts per thread may use the proposed JS_TransferRequest from the bug 551680.

To make only-one-cx-in-request to work with the current FF code base the patch make JS_BeginRequest to suspend context instance that was previously in request. Effectively this requires that requests for different contexts follows stack order but this reflects what happens in FF. With the patch JS_YieldRequest correctly works with no matter how many cx instances are suspended on the current thread. On the other hand I have not changes JS_SuspendRequest to suspend the cx that is active in the request.

Comment on attachment 454656 [details] [diff] [review] v1 The patch passes the tryserver so it is ready for review. But since this may break embeddings I would rather gather feedback first, than ask on js-engine. Note that in FF besides removal very recently introduces and no longer neccessary JS_TransferRequest it was necessary to fix only single place to enforce the one-cx-in-request model. That place is in nsEventListenerService.cpp and the bug was due to the AutoRequest destructor running after nsIThreadJSContextStack::Pop.

Brendan: feedback ping - this simplify fixing the request model violations that must be dealt with in the presence of the conservative GC.

This is a rebased version of v1. Without this patch I cannot fix the request model violations that I have observed when working on the bug 574313 via just adding missing JSAutoRquest. Those could produce dead-lock prone multiple cx in requests.

Brendan: please let me know if you do not have time to review this soon.

Comment on attachment 457282 [details] [diff] [review] v1 Ducking so mrbkap gets this. /be

Comment on attachment 457282 [details] [diff] [review] v1 Wow, this is so much cleaner. I love it!

In http://hg.mozilla.org/tracemonkey/rev/70257f457819 I landed a patch for bug 580578 that wrongfully refers to this bug in its commit message.

http://hg.mozilla.org/tracemonkey/rev/da869ecdb83f - followup to fix sporadic failures in js1_8_5/extensions/worker-fib.js. The fix was to use in jslock.cpp cx->thread->requestContext, not cx->requestDepth, when claiming titles. This is necessary as now JS_BeginRequest suspends another cx request for the current thread without checking for waiting titles.

http://hg.mozilla.org/mozilla-central/rev/3b1c3f0e98d8 (this is fixed on m-c now).