Bug 864957 - Consolidate locks used to avoid operation callback related races, r=djvj,luke.

js/src/ion/AsmJSLink.cpp

@@ -261,7 +261,7 @@ AsmJSActivation::AsmJSActivation(JSContext *cx, const AsmJSModule &module)
 
     prev_ = cx_->runtime->mainThread.asmJSActivationStack_;
 
-    PerThreadData::AsmJSActivationStackLock lock(cx_->runtime->mainThread);
+    JSRuntime::AutoLockForOperationCallback lock(cx_->runtime);
     cx_->runtime->mainThread.asmJSActivationStack_ = this;
 
     (void) errorRejoinSP_;  // squelch GCC warning
@@ -274,7 +274,7 @@ AsmJSActivation::~AsmJSActivation()
 
     JS_ASSERT(cx_->runtime->mainThread.asmJSActivationStack_ == this);
 
-    PerThreadData::AsmJSActivationStackLock lock(cx_->runtime->mainThread);
+    JSRuntime::AutoLockForOperationCallback lock(cx_->runtime);
     cx_->runtime->mainThread.asmJSActivationStack_ = prev_;
 }
 

js/src/ion/AsmJSSignalHandlers.cpp

@@ -976,7 +976,7 @@ void
 js::TriggerOperationCallbackForAsmJSCode(JSRuntime *rt)
 {
 #if defined(JS_ASMJS)
-    PerThreadData::AsmJSActivationStackLock lock(rt->mainThread);
+    JS_ASSERT(rt->currentThreadOwnsOperationCallbackLock());
 
     AsmJSActivation *activation = rt->mainThread.asmJSActivationStackFromAnyThread();
     if (!activation)

js/src/jsapi.cpp

@@ -709,46 +709,20 @@ PerThreadData::PerThreadData(JSRuntime *runtime)
     ionTop(NULL),
     ionJSContext(NULL),
     ionStackLimit(0),
-#ifdef JS_THREADSAFE
-    ionStackLimitLock_(NULL),
-#endif
     ionActivation(NULL),
     asmJSActivationStack_(NULL),
-#ifdef JS_THREADSAFE
-    asmJSActivationStackLock_(NULL),
-#endif
     suppressGC(0)
 {}
 
-bool
-PerThreadData::init()
-{
-#ifdef JS_THREADSAFE
-    ionStackLimitLock_ = PR_NewLock();
-    if (!ionStackLimitLock_)
-        return false;
-
-    asmJSActivationStackLock_ = PR_NewLock();
-    if (!asmJSActivationStackLock_)
-        return false;
-#endif
-    return true;
-}
-
-PerThreadData::~PerThreadData()
-{
-#ifdef JS_THREADSAFE
-    if (ionStackLimitLock_)
-        PR_DestroyLock(ionStackLimitLock_);
-
-    if (asmJSActivationStackLock_)
-        PR_DestroyLock(asmJSActivationStackLock_);
-#endif
-}
-
 JSRuntime::JSRuntime(JSUseHelperThreads useHelperThreads)
   : mainThread(this),
     interrupt(0),
+#ifdef JS_THREADSAFE
+    operationCallbackLock(NULL),
+#ifdef DEBUG
+    operationCallbackOwner(NULL),
+#endif
+#endif
     atomsCompartment(NULL),
     systemZone(NULL),
     numCompartments(0),
@@ -941,10 +915,11 @@ JSRuntime::init(uint32_t maxbytes)
 {
 #ifdef JS_THREADSAFE
     ownerThread_ = PR_GetCurrentThread();
-#endif
 
-    if (!mainThread.init())
+    operationCallbackLock = PR_NewLock();
+    if (!operationCallbackLock)
         return false;
+#endif
 
     js::TlsPerThreadData.set(&mainThread);
 
@@ -1019,6 +994,10 @@ JSRuntime::~JSRuntime()
 {
 #ifdef JS_THREADSAFE
     clearOwnerThread();
+
+    JS_ASSERT(!operationCallbackOwner);
+    if (operationCallbackLock)
+        PR_DestroyLock(operationCallbackLock);
 #endif
 
     /*
@@ -3122,7 +3101,7 @@ JS_SetNativeStackQuota(JSRuntime *rt, size_t stackSize)
     // ionStackLimit, then update it so that it reflects the new nativeStacklimit.
 #ifdef JS_ION
     {
-        PerThreadData::IonStackLimitLock lock(rt->mainThread);
+        JSRuntime::AutoLockForOperationCallback lock(rt);
         if (rt->mainThread.ionStackLimit != uintptr_t(-1))
             rt->mainThread.ionStackLimit = rt->mainThread.nativeStackLimit;
     }

js/src/jscntxt.cpp

@@ -145,6 +145,8 @@ JSRuntime::sizeOfIncludingThis(JSMallocSizeOfFun mallocSizeOf, JS::RuntimeSizes
 void
 JSRuntime::triggerOperationCallback()
 {
+    AutoLockForOperationCallback lock(this);
+
     /*
      * Invalidate ionTop to trigger its over-recursion check. Note this must be
      * set before interrupt, to avoid racing with js_InvokeOperationCallback,

js/src/jscntxt.h

@@ -486,35 +486,7 @@ class PerThreadData : public js::PerThreadDataFriendFields
     JSContext           *ionJSContext;
     uintptr_t            ionStackLimit;
 
-# ifdef JS_THREADSAFE
+    inline void setIonStackLimit(uintptr_t limit);
-    /*
-     * Synchronizes setting of ionStackLimit so signals by triggerOperationCallback don't
-     * get lost.
-     */
-    PRLock *ionStackLimitLock_;
-
-    class IonStackLimitLock {
-        PerThreadData &data_;
-      public:
-        IonStackLimitLock(PerThreadData &data) : data_(data) {
-            JS_ASSERT(data_.ionStackLimitLock_);
-            PR_Lock(data_.ionStackLimitLock_);
-        }
-        ~IonStackLimitLock() {
-            JS_ASSERT(data_.ionStackLimitLock_);
-            PR_Unlock(data_.ionStackLimitLock_);
-        }
-    };
-#else
-    class IonStackLimitLock {
-      public:
-        IonStackLimitLock(PerThreadData &data) {}
-    };
-# endif
-    void setIonStackLimit(uintptr_t limit) {
-        IonStackLimitLock lock(*this);
-        ionStackLimit = limit;
-    }
 
     /*
      * This points to the most recent Ion activation running on the thread.
@@ -527,40 +499,19 @@ class PerThreadData : public js::PerThreadDataFriendFields
      * running asm.js without requiring dynamic polling operations in the
      * generated code. Since triggerOperationCallback may run on a separate
      * thread than the JSRuntime's owner thread all reads/writes must be
-     * synchronized (by asmJSActivationStackLock_).
+     * synchronized (by rt->operationCallbackLock).
      */
   private:
     friend class js::AsmJSActivation;
 
-    /* See AsmJSActivation comment. */
+    /* See AsmJSActivation comment. Protected by rt->operationCallbackLock. */
     js::AsmJSActivation *asmJSActivationStack_;
 
-# ifdef JS_THREADSAFE
-    /* Synchronizes pushing/popping with triggerOperationCallback. */
-    PRLock *asmJSActivationStackLock_;
-# endif
-
   public:
     static unsigned offsetOfAsmJSActivationStackReadOnly() {
         return offsetof(PerThreadData, asmJSActivationStack_);
     }
 
-    class AsmJSActivationStackLock {
-# ifdef JS_THREADSAFE
-        PerThreadData &data_;
-      public:
-        AsmJSActivationStackLock(PerThreadData &data) : data_(data) {
-            PR_Lock(data_.asmJSActivationStackLock_);
-        }
-        ~AsmJSActivationStackLock() {
-            PR_Unlock(data_.asmJSActivationStackLock_);
-        }
-# else
-      public:
-        AsmJSActivationStackLock(PerThreadData &) {}
-# endif
-    };
-
     js::AsmJSActivation *asmJSActivationStackFromAnyThread() const {
         return asmJSActivationStack_;
     }
@@ -579,8 +530,6 @@ class PerThreadData : public js::PerThreadDataFriendFields
     int32_t             suppressGC;
 
     PerThreadData(JSRuntime *runtime);
-    ~PerThreadData();
-    bool init();
 
     bool associatedWith(const JSRuntime *rt) { return runtime_ == rt; }
 };
@@ -690,6 +639,55 @@ struct JSRuntime : public JS::shadow::Runtime,
      */
     volatile int32_t    interrupt;
 
+#ifdef JS_THREADSAFE
+  private:
+    /*
+     * Lock taken when triggering the operation callback from another thread.
+     * Protects all data that is touched in this process.
+     */
+    PRLock *operationCallbackLock;
+#ifdef DEBUG
+    PRThread *operationCallbackOwner;
+#endif
+  public:
+#endif // JS_THREADSAFE
+
+    class AutoLockForOperationCallback {
+#ifdef JS_THREADSAFE
+        JSRuntime *rt;
+      public:
+        AutoLockForOperationCallback(JSRuntime *rt MOZ_GUARD_OBJECT_NOTIFIER_PARAM) : rt(rt) {
+            MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+            PR_Lock(rt->operationCallbackLock);
+#ifdef DEBUG
+            rt->operationCallbackOwner = PR_GetCurrentThread();
+#endif
+        }
+        ~AutoLockForOperationCallback() {
+            JS_ASSERT(rt->operationCallbackOwner == PR_GetCurrentThread());
+#ifdef DEBUG
+            rt->operationCallbackOwner = NULL;
+#endif
+            PR_Unlock(rt->operationCallbackLock);
+        }
+#else // JS_THREADSAFE
+      public:
+        AutoLockForOperationCallback(JSRuntime *rt MOZ_GUARD_OBJECT_NOTIFIER_PARAM) {
+            MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+        }
+#endif // JS_THREADSAFE
+
+        MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+    };
+
+    bool currentThreadOwnsOperationCallbackLock() {
+#if defined(JS_THREADSAFE) && defined(DEBUG)
+        return operationCallbackOwner == PR_GetCurrentThread();
+#else
+        return true;
+#endif
+    }
+
     /* Default compartment. */
     JSCompartment       *atomsCompartment;
 
@@ -1318,6 +1316,7 @@ struct JSRuntime : public JS::shadow::Runtime,
     // Used to reset stack limit after a signaled interrupt (i.e. ionStackLimit_ = -1)
     // has been noticed by Ion/Baseline.
     void resetIonStackLimit() {
+        AutoLockForOperationCallback lock(this);
         mainThread.setIonStackLimit(mainThread.nativeStackLimit);
     }
 
@@ -2035,6 +2034,13 @@ enum ErrorArgumentsType {
     ArgumentsAreASCII
 };
 
+inline void
+PerThreadData::setIonStackLimit(uintptr_t limit)
+{
+    JS_ASSERT(runtime_->currentThreadOwnsOperationCallbackLock());
+    ionStackLimit = limit;
+}
+
 } /* namespace js */
 
 #ifdef va_start