Bug 1263595 - Avoid deadlock between the JIT and the gecko profiler o…

…n win64. (r=froydnj)

dom/indexedDB/ActorsParent.cpp

@@ -24179,6 +24179,10 @@ NormalJSContext::Init()
     return false;
   }
 
+  // Let everyone know that we might be able to call JS. This alerts the
+  // profiler about certain possible deadlocks.
+  NS_GetCurrentThread()->SetCanInvokeJS(true);
+
   // Not setting this will cause JS_CHECK_RECURSION to report false positives.
   JS_SetNativeStackQuota(mContext, 128 * sizeof(size_t) * 1024);
 

js/src/jit/ExecutableAllocatorWin.cpp

@@ -25,6 +25,10 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifdef MOZ_STACKWALKING
+#include "mozilla/StackWalk_windows.h"
+#endif
+
 #include "mozilla/WindowsVersion.h"
 
 #include "jsfriendapi.h"
@@ -165,14 +169,37 @@ RegisterExecutableMemory(void* p, size_t bytes, size_t pageSize)
     if (!VirtualProtect(p, pageSize, PAGE_EXECUTE_READ, &oldProtect))
         return false;
 
-    return RtlAddFunctionTable(&r->runtimeFunction, 1, reinterpret_cast<DWORD64>(p));
+    // XXX NB: The profiler believes this function is only called from the main
+    // thread. If that ever becomes untrue, SPS must be updated immediately.
+#ifdef MOZ_STACKWALKING
+    AcquireStackWalkWorkaroundLock();
+#endif
+
+    bool success = RtlAddFunctionTable(&r->runtimeFunction, 1, reinterpret_cast<DWORD64>(p));
+
+#ifdef MOZ_STACKWALKING
+    ReleaseStackWalkWorkaroundLock();
+#endif
+
+    return success;
 }
 
 static void
 UnregisterExecutableMemory(void* p, size_t bytes, size_t pageSize)
 {
     ExceptionHandlerRecord* r = reinterpret_cast<ExceptionHandlerRecord*>(p);
+
+    // XXX NB: The profiler believes this function is only called from the main
+    // thread. If that ever becomes untrue, SPS must be updated immediately.
+#ifdef MOZ_STACKWALKING
+    AcquireStackWalkWorkaroundLock();
+#endif
+
     RtlDeleteFunctionTable(&r->runtimeFunction);
+
+#ifdef MOZ_STACKWALKING
+    ReleaseStackWalkWorkaroundLock();
+#endif
 }
 #endif
 

mozglue/misc/StackWalk.cpp

@@ -188,6 +188,7 @@ StackWalkInitCriticalAddress()
 #include <stdio.h>
 #include <malloc.h>
 #include "mozilla/ArrayUtils.h"
+#include "mozilla/StackWalk_windows.h"
 
 #include <imagehlp.h>
 // We need a way to know if we are building for WXP (or later), as if we are, we
@@ -441,6 +442,49 @@ WalkStackMain64(struct WalkStackData* aData)
   }
 }
 
+// The JIT needs to allocate executable memory. Because of the inanity of
+// the win64 APIs, this requires locks that stalk walkers also need. Provide
+// another lock to allow synchronization around these resources.
+#ifdef _M_AMD64
+
+struct CriticalSectionAutoInitializer {
+    CRITICAL_SECTION lock;
+
+    CriticalSectionAutoInitializer() {
+      InitializeCriticalSection(&lock);
+    }
+};
+
+static CriticalSectionAutoInitializer gWorkaroundLock;
+
+#endif // _M_AMD64
+
+MFBT_API void
+AcquireStackWalkWorkaroundLock()
+{
+#ifdef _M_AMD64
+  EnterCriticalSection(&gWorkaroundLock.lock);
+#endif
+}
+
+MFBT_API bool
+TryAcquireStackWalkWorkaroundLock()
+{
+#ifdef _M_AMD64
+  return TryEnterCriticalSection(&gWorkaroundLock.lock);
+#else
+  return true;
+#endif
+}
+
+MFBT_API void
+ReleaseStackWalkWorkaroundLock()
+{
+#ifdef _M_AMD64
+  LeaveCriticalSection(&gWorkaroundLock.lock);
+#endif
+}
+
 static unsigned int WINAPI
 WalkStackThread(void* aData)
 {

mozglue/misc/StackWalk_windows.h

@@ -0,0 +1,21 @@
+#ifndef mozilla_StackWalk_windows_h
+#define mozilla_StackWalk_windows_h
+
+#include "mozilla/Types.h"
+
+/**
+ * Allow stack walkers to work around the egregious win64 dynamic lookup table
+ * list API by locking around SuspendThread to avoid deadlock.
+ *
+ * See comment in StackWalk.cpp
+ */
+MFBT_API void
+AcquireStackWalkWorkaroundLock();
+
+MFBT_API bool
+TryAcquireStackWalkWorkaroundLock();
+
+MFBT_API void
+ReleaseStackWalkWorkaroundLock();
+
+#endif // mozilla_StackWalk_windows_h

mozglue/misc/moz.build

@@ -7,6 +7,7 @@ EXPORTS.mozilla += [
 
 if CONFIG['OS_ARCH'] == 'WINNT':
     EXPORTS.mozilla += [
+        'StackWalk_windows.h',
         'TimeStamp_windows.h',
     ]
 

netwerk/base/ProxyAutoConfig.cpp

@@ -761,6 +761,8 @@ ProxyAutoConfig::SetupJS()
   if (mPACScript.IsEmpty())
     return NS_ERROR_FAILURE;
 
+  NS_GetCurrentThread()->SetCanInvokeJS(true);
+
   mJSContext = JSContextWrapper::Create();
   if (!mJSContext)
     return NS_ERROR_FAILURE;

tools/profiler/core/ThreadInfo.cpp

@@ -7,6 +7,8 @@
 #include "ThreadInfo.h"
 #include "ThreadProfile.h"
 
+#include "mozilla/DebugOnly.h"
+
 ThreadInfo::ThreadInfo(const char* aName, int aThreadId,
                        bool aIsMainThread, PseudoStack* aPseudoStack,
                        void* aStackTop)
@@ -50,3 +52,20 @@ ThreadInfo::SetPendingDelete()
   }
 }
 
+bool
+ThreadInfo::CanInvokeJS() const
+{
+#ifdef SPS_STANDALONE
+  return false;
+#else
+  nsIThread* thread = GetThread();
+  if (!thread) {
+    MOZ_ASSERT(IsMainThread());
+    return true;
+  }
+  bool result;
+  mozilla::DebugOnly<nsresult> rv = thread->GetCanInvokeJS(&result);
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  return result;
+#endif
+}

tools/profiler/core/ThreadInfo.h

@@ -35,7 +35,11 @@ class ThreadInfo {
    * May be null for the main thread if the profiler was started during startup
    */
   nsIThread* GetThread() const { return mThread.get(); }
+
 #endif
+
+  bool CanInvokeJS() const;
+
  private:
   char* mName;
   int mThreadId;

tools/profiler/core/ThreadProfile.h

@@ -49,6 +49,9 @@ class ThreadProfile
 #ifndef SPS_STANDALONE
   ThreadResponsiveness* GetThreadResponsiveness() { return &mRespInfo; }
 #endif
+
+  bool CanInvokeJS() const { return mThreadInfo->CanInvokeJS(); }
+
   void SetPendingDelete()
   {
     mPseudoStack = nullptr;

tools/profiler/core/platform-win32.cc

@@ -12,7 +12,7 @@
 //  * Neither the name of Google, Inc. nor the names of its contributors
 //    may be used to endorse or promote products derived from this
 //    software without specific prior written permission.
-// 
+//
 // THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
 // "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
 // LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
@@ -37,6 +37,9 @@
 // Memory profile
 #include "nsMemoryReporterManager.h"
 
+#include "mozilla/StackWalk_windows.h"
+
+
 class PlatformData {
  public:
   // Get a handle to the calling thread. This is the thread that we are
@@ -101,7 +104,7 @@ class SamplerThread : public Thread {
     } else {
       ASSERT(instance_->interval_ == sampler->interval());
     }
-  } 
+  }
 
   static void StopSampler() {
     instance_->Join();
@@ -188,6 +191,29 @@ class SamplerThread : public Thread {
     if (SuspendThread(profiled_thread) == kSuspendFailed)
       return;
 
+    // Threads that may invoke JS require extra attention. Since, on windows,
+    // the jits also need to modify the same dynamic function table that we need
+    // to get a stack trace, we have to be wary of that to avoid deadlock.
+    //
+    // When embedded in Gecko, for threads that aren't the main thread,
+    // CanInvokeJS consults an unlocked value in the nsIThread, so we must
+    // consult this after suspending the profiled thread to avoid racing
+    // against a value change.
+    if (thread_profile->CanInvokeJS()) {
+      if (!TryAcquireStackWalkWorkaroundLock()) {
+        ResumeThread(profiled_thread);
+        return;
+      }
+
+      // It is safe to immediately drop the lock. We only need to contend with
+      // the case in which the profiled thread held needed system resources.
+      // If the profiled thread had held those resources, the trylock would have
+      // failed. Anyone else who grabs those resources will continue to make
+      // progress, since those threads are not suspended. Because of this,
+      // we cannot deadlock with them, and should let them run as they please.
+      ReleaseStackWalkWorkaroundLock();
+    }
+
     // Using only CONTEXT_CONTROL is faster but on 64-bit it causes crashes in
     // RtlVirtualUnwind (see bug 1120126) so we set all the flags.
 #if V8_HOST_ARCH_X64

xpcom/base/CycleCollectedJSContext.cpp

@@ -512,6 +512,8 @@ CycleCollectedJSContext::Initialize(JSContext* aParentContext,
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
+  NS_GetCurrentThread()->SetCanInvokeJS(true);
+
   if (!JS_AddExtraGCRootsTracer(mJSContext, TraceBlackJS, this)) {
     MOZ_CRASH("JS_AddExtraGCRootsTracer failed");
   }

xpcom/threads/HangMonitor.cpp

@@ -16,6 +16,9 @@
 #include "mozilla/UniquePtr.h"
 #include "nsReadableUtils.h"
 #include "mozilla/StackWalk.h"
+#ifdef _WIN64
+#include "mozilla/StackWalk_windows.h"
+#endif
 #include "nsThreadUtils.h"
 #include "nsXULAppAPI.h"
 
@@ -146,7 +149,19 @@ GetChromeHangReport(Telemetry::ProcessedStack& aStack,
   // so allocate ahead of time
   std::vector<uintptr_t> rawStack;
   rawStack.reserve(MAX_CALL_STACK_PCS);
+
+  // Workaround possible deadlock where the main thread is running a
+  // long-standing JS job, and happens to be in the JIT allocator when we
+  // suspend it. Since, on win 64, this requires holding a process lock that
+  // MozStackWalk requires, take this "workaround lock" to avoid deadlock.
+#ifdef _WIN64
+  AcquireStackWalkWorkaroundLock();
+#endif
   DWORD ret = ::SuspendThread(winMainThreadHandle);
+#ifdef _WIN64
+  ReleaseStackWalkWorkaroundLock();
+#endif
+
   if (ret == -1) {
     return;
   }

xpcom/threads/LazyIdleThread.cpp

@@ -458,6 +458,19 @@ LazyIdleThread::GetPRThread(PRThread** aPRThread)
   return NS_ERROR_NOT_AVAILABLE;
 }
 
+NS_IMETHODIMP
+LazyIdleThread::GetCanInvokeJS(bool* aCanInvokeJS)
+{
+  *aCanInvokeJS = false;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+LazyIdleThread::SetCanInvokeJS(bool aCanInvokeJS)
+{
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 NS_IMETHODIMP
 LazyIdleThread::AsyncShutdown()
 {

xpcom/threads/nsIThread.idl

@@ -26,6 +26,14 @@ interface nsIThread : nsIEventTarget
    */
   [noscript] readonly attribute PRThread PRThread;
 
+  /**
+   * @returns
+   *  Whether or not this thread may call into JS. Used in the profiler
+   *  to avoid some unnecessary locking.
+   */
+  [noscript] attribute boolean CanInvokeJS;
+
+
   /**
    * Shutdown the thread.  This method prevents further dispatch of events to
    * the thread, and it causes any pending events to run to completion before

xpcom/threads/nsThread.cpp

@@ -596,6 +596,7 @@ nsThread::nsThread(MainThreadFlag aMainThread, uint32_t aStackSize)
   , mShutdownRequired(false)
   , mEventsAreDoomed(false)
   , mIsMainThread(aMainThread)
+  , mCanInvokeJS(false)
 {
 }
 
@@ -800,6 +801,20 @@ nsThread::GetPRThread(PRThread** aResult)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsThread::GetCanInvokeJS(bool* aResult)
+{
+  *aResult = mCanInvokeJS;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsThread::SetCanInvokeJS(bool aCanInvokeJS)
+{
+  mCanInvokeJS = aCanInvokeJS;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsThread::AsyncShutdown()
 {

xpcom/threads/nsThread.h

@@ -222,6 +222,9 @@ class nsThread
   // Set to true when events posted to this thread will never run.
   bool mEventsAreDoomed;
   MainThreadFlag mIsMainThread;
+
+  // Set to true if this thread creates a JSRuntime.
+  bool mCanInvokeJS;
 };
 
 #if defined(XP_UNIX) && !defined(ANDROID) && !defined(DEBUG) && HAVE_UALARM \