Bug 1233818 part 6 - Make InterruptRunningJitCode non-reentrant. r=luke
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 23 Dec 2015 11:28:54 +0100
changeset 277381 011ba20fcaceadf3c2956e9844bd3abdaf786727
parent 277380 2267d84b2a9c40e897249e98a8303c551727e171
child 277382 ed06bc78715dfa6725bcc8a4ee55f5e90918a595
push id69479
push userjandemooij@gmail.com
push dateWed, 23 Dec 2015 10:37:01 +0000
treeherdermozilla-inbound@ed06bc78715d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1233818
milestone46.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 1233818 part 6 - Make InterruptRunningJitCode non-reentrant. r=luke
js/src/asmjs/AsmJSSignalHandlers.cpp
js/src/jit/Ion.cpp
js/src/vm/Runtime.cpp
js/src/vm/Runtime.h
--- a/js/src/asmjs/AsmJSSignalHandlers.cpp
+++ b/js/src/asmjs/AsmJSSignalHandlers.cpp
@@ -1210,18 +1210,20 @@ RedirectJitCodeToInterruptCheck(JSRuntim
 //  - defaults to nostop and noprint on gdb/lldb so that noone is bothered
 // SIGVTALRM a relative of SIGALRM, so intended for user code, but, unlike
 // SIGALRM, not used anywhere else in Mozilla.
 static const int sInterruptSignal = SIGVTALRM;
 
 static void
 JitInterruptHandler(int signum, siginfo_t* info, void* context)
 {
-    if (JSRuntime* rt = RuntimeForCurrentThread())
+    if (JSRuntime* rt = RuntimeForCurrentThread()) {
         RedirectJitCodeToInterruptCheck(rt, (CONTEXT*)context);
+        rt->finishHandlingJitInterrupt();
+    }
 }
 #endif
 
 bool
 js::EnsureSignalHandlersInstalled(JSRuntime* rt)
 {
 #if defined(XP_DARWIN) && defined(ASMJS_MAY_USE_SIGNAL_HANDLERS_FOR_OOB)
     // On OSX, each JSRuntime gets its own handler thread.
@@ -1318,21 +1320,27 @@ js::EnsureSignalHandlersInstalled(JSRunt
 void
 js::InterruptRunningJitCode(JSRuntime* rt)
 {
     // If signal handlers weren't installed, then Ion and asm.js emit normal
     // interrupt checks and don't need asynchronous interruption.
     if (!rt->canUseSignalHandlers())
         return;
 
+    // Do nothing if we're already handling an interrupt here, to avoid races
+    // below and in JitRuntime::patchIonBackedges.
+    if (!rt->startHandlingJitInterrupt())
+        return;
+
     // If we are on runtime's main thread, then: pc is not in asm.js code (so
     // nothing to do for asm.js) and we can patch Ion backedges without any
     // special synchronization.
     if (rt == RuntimeForCurrentThread()) {
         RedirectIonBackedgesToInterruptCheck(rt);
+        rt->finishHandlingJitInterrupt();
         return;
     }
 
     // We are not on the runtime's main thread, so to do 1 and 2 above, we need
     // to halt the runtime's main thread first.
 #if defined(XP_WIN)
     // On Windows, we can simply suspend the main thread and work directly on
     // its context from this thread. SuspendThread can sporadically fail if the
@@ -1343,16 +1351,17 @@ js::InterruptRunningJitCode(JSRuntime* r
         CONTEXT context;
         context.ContextFlags = CONTEXT_CONTROL;
         if (GetThreadContext(thread, &context)) {
             if (RedirectJitCodeToInterruptCheck(rt, &context))
                 SetThreadContext(thread, &context);
         }
         ResumeThread(thread);
     }
+    rt->finishHandlingJitInterrupt();
 #else
     // On Unix, we instead deliver an async signal to the main thread which
     // halts the thread and callers our JitInterruptHandler (which has already
     // been installed by EnsureSignalHandlersInstalled).
     pthread_t thread = (pthread_t)rt->ownerThreadNative();
     pthread_kill(thread, sInterruptSignal);
 #endif
 }
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -353,18 +353,28 @@ JitRuntime::freeOsrTempData()
 {
     js_free(osrTempData_);
     osrTempData_ = nullptr;
 }
 
 void
 JitRuntime::patchIonBackedges(JSRuntime* rt, BackedgeTarget target)
 {
-    MOZ_ASSERT_IF(target == BackedgeLoopHeader, preventBackedgePatching_);
-    MOZ_ASSERT_IF(target == BackedgeInterruptCheck, !preventBackedgePatching_);
+    if (target == BackedgeLoopHeader) {
+        // We must be on the main thread. The caller must use
+        // AutoPreventBackedgePatching to ensure we don't reenter.
+        MOZ_ASSERT(preventBackedgePatching_);
+        MOZ_ASSERT(CurrentThreadCanAccessRuntime(rt));
+    } else {
+        // We must be called from InterruptRunningJitCode, or a signal handler
+        // triggered there. rt->handlingJitInterrupt() ensures we can't reenter
+        // this code.
+        MOZ_ASSERT(!preventBackedgePatching_);
+        MOZ_ASSERT(rt->handlingJitInterrupt());
+    }
 
     backedgeExecAlloc_.makeAllWritable();
 
     // Patch all loop backedges in Ion code so that they either jump to the
     // normal loop header or to an interrupt handler each time they run.
     for (InlineListIterator<PatchableBackedge> iter(backedgeList_.begin());
          iter != backedgeList_.end();
          iter++)
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -141,16 +141,17 @@ JSRuntime::JSRuntime(JSRuntime* parentRu
     asyncStackForNewActivations(this),
     asyncCauseForNewActivations(this),
     asyncCallIsExplicit(false),
     entryMonitor(nullptr),
     parentRuntime(parentRuntime),
     interrupt_(false),
     telemetryCallback(nullptr),
     handlingSegFault(false),
+    handlingJitInterrupt_(false),
     interruptCallback(nullptr),
     exclusiveAccessLock(nullptr),
     exclusiveAccessOwner(nullptr),
     mainThreadHasExclusiveAccess(false),
     numExclusiveThreads(0),
     numCompartments(0),
     localeCallbacks(nullptr),
     defaultLocale(nullptr),
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -840,16 +840,35 @@ struct JSRuntime : public JS::shadow::Ru
     void* addressOfInterruptUint32() {
         static_assert(sizeof(interrupt_) == sizeof(uint32_t), "Assumed by JIT callers");
         return &interrupt_;
     }
 
     // Set when handling a segfault in the asm.js signal handler.
     bool handlingSegFault;
 
+  private:
+    // Set when we're handling an interrupt of JIT/asm.js code in
+    // InterruptRunningJitCode.
+    mozilla::Atomic<bool> handlingJitInterrupt_;
+
+  public:
+    bool startHandlingJitInterrupt() {
+        // Return true if we changed handlingJitInterrupt_ from
+        // false to true.
+        return handlingJitInterrupt_.compareExchange(false, true);
+    }
+    void finishHandlingJitInterrupt() {
+        MOZ_ASSERT(handlingJitInterrupt_);
+        handlingJitInterrupt_ = false;
+    }
+    bool handlingJitInterrupt() const {
+        return handlingJitInterrupt_;
+    }
+
     JSInterruptCallback interruptCallback;
 
 #ifdef DEBUG
     void assertCanLock(js::RuntimeLock which);
 #else
     void assertCanLock(js::RuntimeLock which) {}
 #endif