Leaving outermost request should js_LeaveTrace (480301, r=brendan).
authorjorendorff
Mon, 20 Apr 2009 18:22:00 -0700
changeset 27577 5d0af2376447781606f9870022955c395f6eb779
parent 27576 c0f9b256e47490092e545c56c7b8aebd3649219f
child 27578 e0e3d87da9add6b0ef434d489778346a4a9ed36b
push id6623
push userrsayre@mozilla.com
push dateTue, 21 Apr 2009 18:35:23 +0000
treeherdermozilla-central@30ac20416be4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbrendan
bugs480301
milestone1.9.2a1pre
Leaving outermost request should js_LeaveTrace (480301, r=brendan).
js/src/jsapi.cpp
js/src/jscntxt.cpp
js/src/jscntxt.h
js/src/jslock.cpp
js/src/jstracer.cpp
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -948,16 +948,18 @@ JS_EndRequest(JSContext *cx)
 #ifdef JS_THREADSAFE
     JSRuntime *rt;
 
     CHECK_REQUEST(cx);
     JS_ASSERT(CURRENT_THREAD_IS_ME(cx->thread));
     JS_ASSERT(cx->requestDepth > 0);
     JS_ASSERT(cx->outstandingRequests > 0);
     if (cx->requestDepth == 1) {
+        js_LeaveTrace(cx);  /* for GC safety */
+
         /* Lock before clearing to interlock with ClaimScope, in jslock.c. */
         rt = cx->runtime;
         JS_LOCK_GC(rt);
         cx->requestDepth = 0;
         cx->outstandingRequests--;
 
         js_ShareWaitingTitles(cx);
         js_RevokeGCLocalFreeLists(cx);
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -846,16 +846,24 @@ js_WaitForGC(JSRuntime *rt)
 uint32
 js_DiscountRequestsForGC(JSContext *cx)
 {
     uint32 requestDebit;
 
     JS_ASSERT(cx->thread);
     JS_ASSERT(cx->runtime->gcThread != cx->thread);
 
+#ifdef JS_TRACER
+    if (JS_ON_TRACE(cx)) {
+        JS_UNLOCK_GC(cx->runtime);
+        js_LeaveTrace(cx);
+        JS_LOCK_GC(cx->runtime);
+    }
+#endif
+
     requestDebit = js_CountThreadRequests(cx);
     if (requestDebit != 0) {
         JSRuntime *rt = cx->runtime;
         JS_ASSERT(requestDebit <= rt->requestCount);
         rt->requestCount -= requestDebit;
         if (rt->requestCount == 0)
             JS_NOTIFY_REQUEST_DONE(rt);
     }
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -173,16 +173,21 @@ struct JSTraceMonitor {
     CLS(nanojit::Fragmento) reFragmento;
 
     /* Keep a list of recorders we need to abort on cache flush. */
     CLS(TraceRecorder)      abortStack;
 };
 
 typedef struct InterpStruct InterpStruct;
 
+/*
+ * N.B. JS_ON_TRACE(cx) is true if JIT code is on the stack in the current
+ * thread, regardless of whether cx is the context in which that trace is
+ * executing.  cx must be a context on the current thread.
+ */
 #ifdef JS_TRACER
 # define JS_ON_TRACE(cx)            (JS_TRACE_MONITOR(cx).onTrace)
 #else
 # define JS_ON_TRACE(cx)            JS_FALSE
 #endif
 
 #ifdef DEBUG
 # define JS_EVAL_CACHE_METERING     1
@@ -225,16 +230,21 @@ struct JSThreadData {
      * The GSN cache is per thread since even multi-cx-per-thread embeddings
      * do not interleave js_GetSrcNote calls.
      */
     JSGSNCache          gsnCache;
 
     /* Property cache for faster call/get/set invocation. */
     JSPropertyCache     propertyCache;
 
+/*
+ * N.B. JS_ON_TRACE(cx) is true if JIT code is on the stack in the current
+ * thread, regardless of whether cx is the context in which that trace is
+ * executing.  cx must be a context on the current thread.
+ */
 #ifdef JS_TRACER
     /* Trace-tree JIT recorder/interpreter state. */
     JSTraceMonitor      traceMonitor;
 #endif
 
     /* Lock-free hashed lists of scripts created by eval to garbage-collect. */
     JSScript            *scriptsToGC[JS_EVAL_CACHE_SIZE];
 
--- a/js/src/jslock.cpp
+++ b/js/src/jslock.cpp
@@ -623,16 +623,24 @@ ClaimTitle(JSTitle *title, JSContext *cx
         }
 
         /*
          * Discount all the requests running on the current thread so a
          * possible GC can proceed on another thread while we wait on
          * rt->titleSharingDone.
          */
         requestDebit = js_DiscountRequestsForGC(cx);
+        if (title->ownercx != ownercx) {
+            /*
+             * js_DiscountRequestsForGC released and reacquired the GC lock,
+             * and the title was taken or shared. Start over.
+             */
+            js_RecountRequestsAfterGC(rt, requestDebit);
+            continue;
+        }
 
         /*
          * We know that some other thread's context owns title, which is now
          * linked onto rt->titleSharingTodo, awaiting the end of that other
          * thread's request. So it is safe to wait on rt->titleSharingDone.
          * But before waiting, we force the operation callback for that other
          * thread so it can quickly suspend.
          */
@@ -647,17 +655,17 @@ ClaimTitle(JSTitle *title, JSContext *cx
         JS_ASSERT(stat != PR_FAILURE);
 
         js_RecountRequestsAfterGC(rt, requestDebit);
 
         /*
          * Don't clear titleToShare until after we're through waiting on
          * all condition variables protected by rt->gcLock -- that includes
          * rt->titleSharingDone *and* rt->gcDone (hidden in the call to
-         * js_ActivateRequestAfterGC immediately above).
+         * js_RecountRequestsAfterGC immediately above).
          *
          * Otherwise, the GC could easily deadlock with another thread that
          * owns a title wanted by a finalizer.  By keeping cx->titleToShare
          * set till here, we ensure that such deadlocks are detected, which
          * results in the finalized object's title being shared (it must, of
          * course, have other, live objects sharing it).
          */
         cx->thread->titleToShare = NULL;
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -5025,25 +5025,48 @@ js_OverfullFragmento(Fragmento *frago, s
     return (frago->_stats.pages > (maxsz >> NJ_LOG2_PAGE_SIZE));
 }
 
 JS_FORCES_STACK JS_FRIEND_API(void)
 js_DeepBail(JSContext *cx)
 {
     JS_ASSERT(JS_ON_TRACE(cx));
 
+    /*
+     * Exactly one context on the current thread is on trace. Find out which
+     * one. (Most callers cannot guarantee that it's cx.)
+     */
+    JSContext *tracecx = NULL;
+#ifdef JS_THREADSAFE
+    JSCList *head = &cx->thread->contextList;
+    for (JSCList *link = head->next; link != head; link = link->next) {
+        JSContext *acx = CX_FROM_THREAD_LINKS(link);
+        JS_ASSERT(!(acx->requestDepth == 0 && acx->bailExit));
+#else
+    JSContext *acx, *iter = NULL;
+    while ((acx = js_ContextIterator(cx->runtime, JS_TRUE, &iter)) != NULL) {
+#endif
+        if (acx->bailExit) {
+            JS_ASSERT(!tracecx);
+            tracecx = acx;
+#ifndef DEBUG
+            break;
+#endif
+        }
+    }
+
     /* It's a bug if a non-FAIL_STATUS builtin gets here. */
-    JS_ASSERT(cx->bailExit);
-
-    JS_TRACE_MONITOR(cx).onTrace = false;
-    JS_TRACE_MONITOR(cx).prohibitFlush++;
+    JS_ASSERT(tracecx->bailExit);
+
+    JS_TRACE_MONITOR(tracecx).onTrace = false;
+    JS_TRACE_MONITOR(tracecx).prohibitFlush++;
     debug_only_v(printf("Deep bail.\n");)
-    LeaveTree(*cx->interpState, cx->bailExit);
-    cx->bailExit = NULL;
-    cx->interpState->builtinStatus |= JSBUILTIN_BAILED;
+    LeaveTree(*tracecx->interpState, tracecx->bailExit);
+    tracecx->bailExit = NULL;
+    tracecx->interpState->builtinStatus |= JSBUILTIN_BAILED;
 }
 
 JS_REQUIRES_STACK jsval&
 TraceRecorder::argval(unsigned n) const
 {
     JS_ASSERT(n < cx->fp->fun->nargs);
     return cx->fp->argv[n];
 }