bug 730234 - remove GC locking from activities, operation callbacks-r…

…elated code and for code that accesses the JSContext list. r=luke
dothq · Feb 24, 2012 · c5c7623 · c5c7623
1 parent 56ec12d
commit c5c7623
Show file tree

Hide file tree

Showing 14 changed files with 152 additions and 243 deletions.
diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp
@@ -2925,7 +2925,7 @@ WorkerPrivate::Dispatch(WorkerRunnable* aEvent, EventQueue* aQueue)
     }
 
     if (aQueue == &mControlQueue && mJSContext) {
-      JS_TriggerOperationCallback(mJSContext);
+      JS_TriggerOperationCallback(JS_GetRuntime(mJSContext));
     }
 
     mCondVar.Notify();

diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
@@ -1014,8 +1014,6 @@ StartRequest(JSContext *cx)
     if (rt->requestDepth) {
         rt->requestDepth++;
     } else {
-        AutoLockGC lock(rt);
-
         /* Indicate that a request is running. */
         rt->requestDepth = 1;
 
@@ -1034,10 +1032,6 @@ StopRequest(JSContext *cx)
         rt->requestDepth--;
     } else {
         rt->conservativeGC.updateForRequestEnd(rt->suspendCount);
-
-        /* Lock before clearing to interlock with ClaimScope, in jslock.c. */
-        AutoLockGC lock(rt);
-
         rt->requestDepth = 0;
 
         if (rt->activityCallback)
@@ -1307,14 +1301,12 @@ SetOptionsCommon(JSContext *cx, unsigned options)
 JS_PUBLIC_API(uint32_t)
 JS_SetOptions(JSContext *cx, uint32_t options)
 {
-    AutoLockGC lock(cx->runtime);
     return SetOptionsCommon(cx, options);
 }
 
 JS_PUBLIC_API(uint32_t)
 JS_ToggleOptions(JSContext *cx, uint32_t options)
 {
-    AutoLockGC lock(cx->runtime);
     unsigned oldopts = cx->allOptions();
     unsigned newopts = oldopts ^ options;
     return SetOptionsCommon(cx, newopts);
@@ -2896,7 +2888,6 @@ JS_SetGCParameter(JSRuntime *rt, JSGCParamKey key, uint32_t value)
 {
     switch (key) {
       case JSGC_MAX_BYTES: {
-        AutoLockGC lock(rt);
         JS_ASSERT(value >= rt->gcBytes);
         rt->gcMaxBytes = value;
         break;
@@ -5523,20 +5514,8 @@ JS_GetOperationCallback(JSContext *cx)
 }
 
 JS_PUBLIC_API(void)
-JS_TriggerOperationCallback(JSContext *cx)
+JS_TriggerOperationCallback(JSRuntime *rt)
 {
-#ifdef JS_THREADSAFE
-    AutoLockGC lock(cx->runtime);
-#endif
-    cx->runtime->triggerOperationCallback();
-}
-
-JS_PUBLIC_API(void)
-JS_TriggerRuntimeOperationCallback(JSRuntime *rt)
-{
-#ifdef JS_THREADSAFE
-    AutoLockGC lock(rt);
-#endif
     rt->triggerOperationCallback();
 }
 

diff --git a/js/src/jsapi.h b/js/src/jsapi.h
@@ -4504,32 +4504,25 @@ JS_BEGIN_EXTERN_C
 
 /*
  * These functions allow setting an operation callback that will be called
- * from the thread the context is associated with some time after any thread
- * triggered the callback using JS_TriggerOperationCallback(cx).
+ * from the JS thread some time after any thread triggered the callback using
+ * JS_TriggerOperationCallback(rt).
  *
- * In a threadsafe build the engine internally triggers operation callbacks
- * under certain circumstances (i.e. GC and title transfer) to force the
- * context to yield its current request, which the engine always
- * automatically does immediately prior to calling the callback function.
- * The embedding should thus not rely on callbacks being triggered through
- * the external API only.
+ * To schedule the GC and for other activities the engine internally triggers
+ * operation callbacks. The embedding should thus not rely on callbacks being
+ * triggered through the external API only.
  *
  * Important note: Additional callbacks can occur inside the callback handler
  * if it re-enters the JS engine. The embedding must ensure that the callback
  * is disconnected before attempting such re-entry.
  */
-
 extern JS_PUBLIC_API(JSOperationCallback)
 JS_SetOperationCallback(JSContext *cx, JSOperationCallback callback);
 
 extern JS_PUBLIC_API(JSOperationCallback)
 JS_GetOperationCallback(JSContext *cx);
 
 extern JS_PUBLIC_API(void)
-JS_TriggerOperationCallback(JSContext *cx);
-
-extern JS_PUBLIC_API(void)
-JS_TriggerRuntimeOperationCallback(JSRuntime *rt);
+JS_TriggerOperationCallback(JSRuntime *rt);
 
 extern JS_PUBLIC_API(JSBool)
 JS_IsRunning(JSContext *cx);

diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp
@@ -118,7 +118,7 @@ JSRuntime::sizeOfExcludingThis(JSMallocSizeOfFun mallocSizeOf, size_t *normal, s
         *gcMarkerSize = gcMarker.sizeOfExcludingThis(mallocSizeOf);
 }
 
-JS_FRIEND_API(void)
+void
 JSRuntime::triggerOperationCallback()
 {
     /*
@@ -250,49 +250,48 @@ js_DestroyContext(JSContext *cx, JSDestroyContextMode mode)
         }
     }
 
-    JS_LOCK_GC(rt);
     JS_REMOVE_LINK(&cx->link);
     bool last = !rt->hasContexts();
-    if (last || mode == JSDCM_FORCE_GC || mode == JSDCM_MAYBE_GC) {
+    if (last) {
         JS_ASSERT(!rt->gcRunning);
 
 #ifdef JS_THREADSAFE
-        rt->gcHelperThread.waitBackgroundSweepEnd();
-#endif
-        JS_UNLOCK_GC(rt);
-
-        if (last) {
-            /*
-             * Dump remaining type inference results first. This printing
-             * depends on atoms still existing.
-             */
-            {
-                AutoLockGC lock(rt);
-                for (CompartmentsIter c(rt); !c.done(); c.next())
-                    c->types.print(cx, false);
-            }
-
-            /* Unpin all common atoms before final GC. */
-            js_FinishCommonAtoms(cx);
-
-            /* Clear debugging state to remove GC roots. */
-            for (CompartmentsIter c(rt); !c.done(); c.next())
-                c->clearTraps(cx);
-            JS_ClearAllWatchPoints(cx);
-
-            GC(cx, NULL, GC_NORMAL, gcreason::LAST_CONTEXT);
-
-        } else if (mode == JSDCM_FORCE_GC) {
-            GC(cx, NULL, GC_NORMAL, gcreason::DESTROY_CONTEXT);
-        } else if (mode == JSDCM_MAYBE_GC) {
-            JS_MaybeGC(cx);
+        {
+            AutoLockGC lock(rt);
+            rt->gcHelperThread.waitBackgroundSweepEnd();
         }
-        JS_LOCK_GC(rt);
+#endif
+
+        /*
+         * Dump remaining type inference results first. This printing
+         * depends on atoms still existing.
+         */
+        for (CompartmentsIter c(rt); !c.done(); c.next())
+            c->types.print(cx, false);
+
+        /* Unpin all common atoms before final GC. */
+        js_FinishCommonAtoms(cx);
+
+        /* Clear debugging state to remove GC roots. */
+        for (CompartmentsIter c(rt); !c.done(); c.next())
+            c->clearTraps(cx);
+        JS_ClearAllWatchPoints(cx);
+
+        GC(cx, NULL, GC_NORMAL, gcreason::LAST_CONTEXT);
+    } else if (mode == JSDCM_FORCE_GC) {
+        JS_ASSERT(!rt->gcRunning);
+        GC(cx, NULL, GC_NORMAL, gcreason::DESTROY_CONTEXT);
+    } else if (mode == JSDCM_MAYBE_GC) {
+        JS_ASSERT(!rt->gcRunning);
+        JS_MaybeGC(cx);
     }
+
 #ifdef JS_THREADSAFE
-    rt->gcHelperThread.waitBackgroundSweepEnd();
+    {
+        AutoLockGC lock(rt);
+        rt->gcHelperThread.waitBackgroundSweepEnd();
+    }
 #endif
-    JS_UNLOCK_GC(rt);
     Foreground::delete_(cx);
 }
 

diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h
@@ -686,7 +686,7 @@ struct JSRuntime : js::RuntimeFriendFields
      */
     JS_FRIEND_API(void *) onOutOfMemory(void *p, size_t nbytes, JSContext *cx);
 
-    JS_FRIEND_API(void) triggerOperationCallback();
+    void triggerOperationCallback();
 
     void setJitHardening(bool enabled);
     bool getJitHardening() const {
@@ -1247,6 +1247,40 @@ class AutoXMLRooter : private AutoGCRooter {
 # define JS_UNLOCK_GC(rt)
 #endif
 
+class AutoLockGC
+{
+  public:
+    explicit AutoLockGC(JSRuntime *rt = NULL
+                        MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+      : runtime(rt)
+    {
+        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+        if (rt)
+            JS_LOCK_GC(rt);
+    }
+
+    ~AutoLockGC()
+    {
+        if (runtime)
+            JS_UNLOCK_GC(runtime);
+    }
+
+    bool locked() const {
+        return !!runtime;
+    }
+
+    void lock(JSRuntime *rt) {
+        JS_ASSERT(rt);
+        JS_ASSERT(!runtime);
+        runtime = rt;
+        JS_LOCK_GC(rt);
+    }
+
+  private:
+    JSRuntime *runtime;
+    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+};
+
 class AutoUnlockGC {
   private:
     JSRuntime *rt;

diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp
@@ -609,29 +609,6 @@ js::DumpHeapComplete(JSRuntime *rt, FILE *fp)
 
 namespace js {
 
-/* static */ void
-AutoLockGC::LockGC(JSRuntime *rt)
-{
-    JS_ASSERT(rt);
-    JS_LOCK_GC(rt);
-}
-
-/* static */ void
-AutoLockGC::UnlockGC(JSRuntime *rt)
-{
-    JS_ASSERT(rt);
-    JS_UNLOCK_GC(rt);
-}
-
-void
-AutoLockGC::lock(JSRuntime *rt)
-{
-    JS_ASSERT(rt);
-    JS_ASSERT(!runtime);
-    runtime = rt;
-    JS_LOCK_GC(rt);
-}
-
 JS_FRIEND_API(const JSStructuredCloneCallbacks *)
 GetContextStructuredCloneCallbacks(JSContext *cx)
 {
@@ -674,12 +651,6 @@ GetContextOutstandingRequests(const JSContext *cx)
     return cx->outstandingRequests;
 }
 
-JS_FRIEND_API(PRLock *)
-GetRuntimeGCLock(const JSRuntime *rt)
-{
-    return rt->gcLock;
-}
-
 AutoSkipConservativeScan::AutoSkipConservativeScan(JSContext *cx
                                                    MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
   : context(cx)
@@ -726,12 +697,6 @@ IsContextRunningJS(JSContext *cx)
     return !cx->stack.empty();
 }
 
-JS_FRIEND_API(void)
-TriggerOperationCallback(JSRuntime *rt)
-{
-    rt->triggerOperationCallback();
-}
-
 JS_FRIEND_API(const CompartmentVector&)
 GetRuntimeCompartments(JSRuntime *rt)
 {

diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h
@@ -187,8 +187,6 @@ JS_END_EXTERN_C
 
 #ifdef __cplusplus
 
-struct PRLock;
-
 namespace js {
 
 struct ContextFriendFields {
@@ -573,9 +571,6 @@ GetOwnerThread(const JSContext *cx);
 JS_FRIEND_API(unsigned)
 GetContextOutstandingRequests(const JSContext *cx);
 
-JS_FRIEND_API(PRLock *)
-GetRuntimeGCLock(const JSRuntime *rt);
-
 class JS_FRIEND_API(AutoSkipConservativeScan)
 {
   public:
@@ -600,43 +595,11 @@ typedef void
 /*
  * Sets a callback that is run whenever the runtime goes idle - the
  * last active request ceases - and begins activity - when it was
- * idle and a request begins. Note: The callback is called under the
- * GC lock.
+ * idle and a request begins.
  */
 JS_FRIEND_API(void)
 SetActivityCallback(JSRuntime *rt, ActivityCallback cb, void *arg);
 
-class JS_FRIEND_API(AutoLockGC)
-{
-  public:
-    explicit AutoLockGC(JSRuntime *rt = NULL
-                        MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
-      : runtime(rt)
-    {
-        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
-        if (rt)
-            LockGC(rt);
-    }
-
-    ~AutoLockGC()
-    {
-        if (runtime)
-            UnlockGC(runtime);
-    }
-
-    bool locked() const {
-        return !!runtime;
-    }
-    void lock(JSRuntime *rt);
-
-  private:
-    static void LockGC(JSRuntime *rt);
-    static void UnlockGC(JSRuntime *rt);
-
-    JSRuntime *runtime;
-    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
-};
-
 extern JS_FRIEND_API(const JSStructuredCloneCallbacks *)
 GetContextStructuredCloneCallbacks(JSContext *cx);
 
@@ -652,10 +615,6 @@ CallContextDebugHandler(JSContext *cx, JSScript *script, jsbytecode *bc, Value *
 extern JS_FRIEND_API(bool)
 IsContextRunningJS(JSContext *cx);
 
-/* Must be called with GC lock taken. */
-extern JS_FRIEND_API(void)
-TriggerOperationCallback(JSRuntime *rt);
-
 class SystemAllocPolicy;
 typedef Vector<JSCompartment*, 0, SystemAllocPolicy> CompartmentVector;
 extern JS_FRIEND_API(const CompartmentVector&)