Bug 956899 - Use Mutex and ConditionVariable to simplify shell watchdog; r=jandem
☠☠ backed out by 3c86f1e4bbc9 ☠ ☠
authorTerrence Cole <terrence@mozilla.com>
Wed, 25 May 2016 09:41:42 -0700
changeset 340400 67d13c5fcf841e33734c0568fffa9f7686fd264f
parent 340399 18f02d991078d01ae91f2b6c6ed1a6520143f2cd
child 340401 8aa6b27297b6aa94abb63b2430f7879f14dfa86a
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs956899
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 956899 - Use Mutex and ConditionVariable to simplify shell watchdog; r=jandem
js/src/shell/js.cpp
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -74,16 +74,19 @@
 #include "js/GCAPI.h"
 #include "js/Initialization.h"
 #include "js/StructuredClone.h"
 #include "js/TrackedOptimizationInfo.h"
 #include "perf/jsperf.h"
 #include "shell/jsoptparse.h"
 #include "shell/jsshell.h"
 #include "shell/OSObject.h"
+#include "threading/ConditionVariable.h"
+#include "threading/LockGuard.h"
+#include "threading/Mutex.h"
 #include "vm/ArgumentsObject.h"
 #include "vm/Compression.h"
 #include "vm/Debugger.h"
 #include "vm/HelperThreads.h"
 #include "vm/Monitor.h"
 #include "vm/Shape.h"
 #include "vm/SharedArrayObject.h"
 #include "vm/StringBuffer.h"
@@ -162,22 +165,22 @@ struct ShellRuntime
 #ifdef SPIDERMONKEY_PROMISE
     JS::PersistentRootedValue promiseRejectionTrackerCallback;
     JS::PersistentRooted<JobQueue> jobQueue;
 #endif // SPIDERMONKEY_PROMISE
 
     /*
      * Watchdog thread state.
      */
-    PRLock* watchdogLock;
-    PRCondVar* watchdogWakeup;
+    Mutex watchdogLock;
+    ConditionVariable watchdogWakeup;
     PRThread* watchdogThread;
     Maybe<TimeStamp> watchdogTimeout;
 
-    PRCondVar* sleepWakeup;
+    ConditionVariable sleepWakeup;
 
     int exitCode;
     bool quitting;
     bool gotError;
 };
 
 // Shell state set once at startup.
 static bool enableCodeCoverage = false;
@@ -211,19 +214,16 @@ static bool OOM_printAllocationCount = f
 
 // Shell state this is only accessed on the main thread.
 bool jsCachingEnabled = false;
 mozilla::Atomic<bool> jsCacheOpened(false);
 
 static bool
 SetTimeoutValue(JSContext* cx, double t);
 
-static bool
-InitWatchdog(JSRuntime* rt);
-
 static void
 KillWatchdog(JSRuntime *rt);
 
 static bool
 ScheduleWatchdog(JSRuntime* rt, double t);
 
 static void
 CancelExecution(JSRuntime* rt);
@@ -310,20 +310,17 @@ ShellRuntime::ShellRuntime(JSRuntime* rt
     serviceInterrupt(false),
     haveInterruptFunc(false),
     interruptFunc(rt, NullValue()),
     lastWarningEnabled(false),
     lastWarning(rt, NullValue()),
 #ifdef SPIDERMONKEY_PROMISE
     promiseRejectionTrackerCallback(rt, NullValue()),
 #endif // SPIDERMONKEY_PROMISE
-    watchdogLock(nullptr),
-    watchdogWakeup(nullptr),
     watchdogThread(nullptr),
-    sleepWakeup(nullptr),
     exitCode(0),
     quitting(false),
     gotError(false)
 {}
 
 static ShellRuntime*
 GetShellRuntime(JSRuntime *rt)
 {
@@ -2931,22 +2928,16 @@ WorkerMain(void* arg)
 
     sr->isWorker = true;
     JS_SetRuntimePrivate(rt, sr.get());
     JS_SetFutexCanWait(rt);
     JS_SetErrorReporter(rt, my_ErrorReporter);
     JS_InitDestroyPrincipalsCallback(rt, ShellPrincipals::destroy);
     SetWorkerRuntimeOptions(rt);
 
-    if (!InitWatchdog(rt)) {
-        JS_DestroyRuntime(rt);
-        js_delete(input);
-        return;
-    }
-
     JSContext* cx = NewContext(rt);
     if (!cx) {
         JS_DestroyRuntime(rt);
         js_delete(input);
         return;
     }
 
 #ifdef SPIDERMONKEY_PROMISE
@@ -3096,151 +3087,125 @@ Sleep_fn(JSContext* cx, unsigned argc, V
         duration = TimeDuration::FromSeconds(Max(0.0, t_secs));
 
         /* NB: The next condition also filter out NaNs. */
         if (duration > MAX_TIMEOUT_INTERVAL) {
             JS_ReportError(cx, "Excessive sleep interval");
             return false;
         }
     }
-    PR_Lock(sr->watchdogLock);
-    TimeStamp toWakeup = TimeStamp::Now() + duration;
-    for (;;) {
-        PR_WaitCondVar(sr->sleepWakeup, DurationToPRInterval(duration));
-        if (sr->serviceInterrupt)
-            break;
-        auto now = TimeStamp::Now();
-        if (now >= toWakeup)
-            break;
-        duration = toWakeup - now;
-    }
-    PR_Unlock(sr->watchdogLock);
+    {
+        LockGuard<Mutex> guard(sr->watchdogLock);
+        TimeStamp toWakeup = TimeStamp::Now() + duration;
+        for (;;) {
+            sr->sleepWakeup.wait_for(guard, duration);
+            if (sr->serviceInterrupt)
+                break;
+            auto now = TimeStamp::Now();
+            if (now >= toWakeup)
+                break;
+            duration = toWakeup - now;
+        }
+    }
     args.rval().setUndefined();
     return !sr->serviceInterrupt;
 }
 
-static bool
-InitWatchdog(JSRuntime* rt)
-{
-    ShellRuntime* sr = GetShellRuntime(rt);
-    MOZ_ASSERT(!sr->watchdogThread);
-    sr->watchdogLock = PR_NewLock();
-    if (sr->watchdogLock) {
-        sr->watchdogWakeup = PR_NewCondVar(sr->watchdogLock);
-        if (sr->watchdogWakeup) {
-            sr->sleepWakeup = PR_NewCondVar(sr->watchdogLock);
-            if (sr->sleepWakeup)
-                return true;
-            PR_DestroyCondVar(sr->watchdogWakeup);
-        }
-        PR_DestroyLock(sr->watchdogLock);
-    }
-    return false;
-}
-
 static void
 KillWatchdog(JSRuntime* rt)
 {
     ShellRuntime* sr = GetShellRuntime(rt);
     PRThread* thread;
 
-    PR_Lock(sr->watchdogLock);
-    thread = sr->watchdogThread;
-    if (thread) {
-        /*
-         * The watchdog thread is running, tell it to terminate waking it up
-         * if necessary.
-         */
-        sr->watchdogThread = nullptr;
-        PR_NotifyCondVar(sr->watchdogWakeup);
-    }
-    PR_Unlock(sr->watchdogLock);
+    {
+        LockGuard<Mutex> guard(sr->watchdogLock);
+        thread = sr->watchdogThread;
+        if (thread) {
+            /*
+             * The watchdog thread is running, tell it to terminate waking it up
+             * if necessary.
+             */
+            sr->watchdogThread = nullptr;
+            sr->watchdogWakeup.notify_one();
+        }
+    }
     if (thread)
         PR_JoinThread(thread);
-    PR_DestroyCondVar(sr->sleepWakeup);
-    PR_DestroyCondVar(sr->watchdogWakeup);
-    PR_DestroyLock(sr->watchdogLock);
 }
 
 static void
 WatchdogMain(void* arg)
 {
     PR_SetCurrentThreadName("JS Watchdog");
 
     JSRuntime* rt = (JSRuntime*) arg;
     ShellRuntime* sr = GetShellRuntime(rt);
 
-    PR_Lock(sr->watchdogLock);
+    LockGuard<Mutex> guard(sr->watchdogLock);
     while (sr->watchdogThread) {
         auto now = TimeStamp::Now();
         if (sr->watchdogTimeout.isSome() && now >= sr->watchdogTimeout.value()) {
             /*
              * The timeout has just expired. Request an interrupt callback
              * outside the lock.
              */
             sr->watchdogTimeout = Nothing();
-            PR_Unlock(sr->watchdogLock);
-            CancelExecution(rt);
-            PR_Lock(sr->watchdogLock);
+            {
+                UnlockGuard<Mutex> unlock(guard);
+                CancelExecution(rt);
+            }
 
             /* Wake up any threads doing sleep. */
-            PR_NotifyAllCondVar(sr->sleepWakeup);
+            sr->sleepWakeup.notify_all();
         } else {
             if (sr->watchdogTimeout.isSome()) {
                 /*
                  * Time hasn't expired yet. Simulate an interrupt callback
                  * which doesn't abort execution.
                  */
                 JS_RequestInterruptCallback(rt);
             }
 
             TimeDuration sleepDuration = sr->watchdogTimeout.isSome()
                                          ? TimeDuration::FromSeconds(0.1)
                                          : TimeDuration::Forever();
-            mozilla::DebugOnly<PRStatus> status =
-              PR_WaitCondVar(sr->watchdogWakeup, DurationToPRInterval(sleepDuration));
-            MOZ_ASSERT(status == PR_SUCCESS);
+            sr->watchdogWakeup.wait_for(guard, sleepDuration);
         }
     }
-    PR_Unlock(sr->watchdogLock);
 }
 
 static bool
 ScheduleWatchdog(JSRuntime* rt, double t)
 {
     ShellRuntime* sr = GetShellRuntime(rt);
 
     if (t <= 0) {
-        PR_Lock(sr->watchdogLock);
+        LockGuard<Mutex> guard(sr->watchdogLock);
         sr->watchdogTimeout = Nothing();
-        PR_Unlock(sr->watchdogLock);
         return true;
     }
 
     auto interval = TimeDuration::FromSeconds(t);
     auto timeout = TimeStamp::Now() + interval;
-    PR_Lock(sr->watchdogLock);
+    LockGuard<Mutex> guard(sr->watchdogLock);
     if (!sr->watchdogThread) {
         MOZ_ASSERT(sr->watchdogTimeout.isNothing());
         sr->watchdogThread = PR_CreateThread(PR_USER_THREAD,
                                           WatchdogMain,
                                           rt,
                                           PR_PRIORITY_NORMAL,
                                           PR_GLOBAL_THREAD,
                                           PR_JOINABLE_THREAD,
                                           0);
-        if (!sr->watchdogThread) {
-            PR_Unlock(sr->watchdogLock);
+        if (!sr->watchdogThread)
             return false;
-        }
     } else if (sr->watchdogTimeout.isNothing() || timeout < sr->watchdogTimeout.value()) {
-         PR_NotifyCondVar(sr->watchdogWakeup);
+         sr->watchdogWakeup.notify_one();
     }
     sr->watchdogTimeout = Some(timeout);
-    PR_Unlock(sr->watchdogLock);
     return true;
 }
 
 static void
 KillWorkerThreads()
 {
     MOZ_ASSERT_IF(!CanUseExtraThreads(), workerThreads.empty());
 
@@ -7425,19 +7390,16 @@ main(int argc, char** argv, char** envp)
 
     JS_SetNativeStackQuota(rt, gMaxStackSize);
 
     JS::dbg::SetDebuggerMallocSizeOf(rt, moz_malloc_size_of);
 
     if (!offThreadState.init())
         return 1;
 
-    if (!InitWatchdog(rt))
-        return 1;
-
     cx = NewContext(rt);
     if (!cx)
         return 1;
 
 #ifdef SPIDERMONKEY_PROMISE
     sr->jobQueue.init(cx, JobQueue(SystemAllocPolicy()));
     JS::SetEnqueuePromiseJobCallback(rt, ShellEnqueuePromiseJobCallback);
 #endif // SPIDERMONKEY_PROMISE