Bug 1573589. Be more explicit about propagating the "current runnable's global" through nested event loops in workers. r=baku
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 15 Aug 2019 12:51:58 +0000
changeset 488237 04f03c56a77f2f5d2543937bb59d2bd6af136ae7
parent 488236 893922ffdc3028ce282d8017503ebff4ce0bec7a
child 488238 5f30bde4179009fd405f8bd8c924b1f877874d42
push id36437
push userncsoregi@mozilla.com
push dateThu, 15 Aug 2019 19:33:18 +0000
treeherdermozilla-central@44aac6fc3352 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1573589
milestone70.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 1573589. Be more explicit about propagating the "current runnable's global" through nested event loops in workers. r=baku Some worker debugger runnables (the ones that want to evaluate script against a debugger sandbox) depend on the JSContext being in a particular Realm before they run, but don't really store which Realm that should be. Instead of propagating that state via the current Realm of the JSContext across nested event loops, we want to propagate it explicitly. Differential Revision: https://phabricator.services.mozilla.com/D41790
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
dom/workers/WorkerRunnable.cpp
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -3646,16 +3646,18 @@ already_AddRefed<nsIEventTarget> WorkerP
 }
 
 bool WorkerPrivate::RunCurrentSyncLoop() {
   AssertIsOnWorkerThread();
 
   JSContext* cx = GetJSContext();
   MOZ_ASSERT(cx);
 
+  AutoPushEventLoopGlobal eventLoopGlobal(this, cx);
+
   // This should not change between now and the time we finish running this sync
   // loop.
   uint32_t currentLoopIndex = mSyncLoopStack.Length() - 1;
 
   SyncLoopInfo* loopInfo = mSyncLoopStack[currentLoopIndex];
 
   MOZ_ASSERT(loopInfo);
   MOZ_ASSERT(!loopInfo->mHasRun);
@@ -3710,17 +3712,20 @@ bool WorkerPrivate::RunCurrentSyncLoop()
 
     if (normalRunnablesPending) {
       // Make sure the periodic timer is running before we continue.
       SetGCTimerMode(PeriodicTimer);
 
       MOZ_ALWAYS_TRUE(NS_ProcessNextEvent(mThread, false));
 
       // Now *might* be a good time to GC. Let the JS engine make the decision.
-      if (JS::CurrentGlobalOrNull(cx)) {
+      if (GetCurrentEventLoopGlobal()) {
+        // If GetCurrentEventLoopGlobal() is non-null, our JSContext is in a
+        // Realm, so it's safe to try to GC.
+        MOZ_ASSERT(JS::CurrentGlobalOrNull(cx));
         JS_MaybeGC(cx);
       }
     }
   }
 
   // Make sure that the stack didn't change underneath us.
   MOZ_ASSERT(mSyncLoopStack[currentLoopIndex] == loopInfo);
 
@@ -3899,16 +3904,19 @@ void WorkerPrivate::PostMessageToParent(
   }
 }
 
 void WorkerPrivate::EnterDebuggerEventLoop() {
   MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
 
   JSContext* cx = GetJSContext();
   MOZ_ASSERT(cx);
+
+  AutoPushEventLoopGlobal eventLoopGlobal(this, cx);
+
   CycleCollectedJSContext* ccjscx = CycleCollectedJSContext::Get();
 
   uint32_t currentEventLoopLevel = ++data->mDebuggerEventLoopLevel;
 
   while (currentEventLoopLevel <= data->mDebuggerEventLoopLevel) {
     bool debuggerRunnablesPending = false;
 
     {
@@ -3953,17 +3961,20 @@ void WorkerPrivate::EnterDebuggerEventLo
 
       MOZ_ASSERT(runnable);
       static_cast<nsIRunnable*>(runnable)->Run();
       runnable->Release();
 
       ccjscx->PerformDebuggerMicroTaskCheckpoint();
 
       // Now *might* be a good time to GC. Let the JS engine make the decision.
-      if (JS::CurrentGlobalOrNull(cx)) {
+      if (GetCurrentEventLoopGlobal()) {
+        // If GetCurrentEventLoopGlobal() is non-null, our JSContext is in a
+        // Realm, so it's safe to try to GC.
+        MOZ_ASSERT(JS::CurrentGlobalOrNull(cx));
         JS_MaybeGC(cx);
       }
     }
   }
 }
 
 void WorkerPrivate::LeaveDebuggerEventLoop() {
   MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
@@ -4961,10 +4972,25 @@ WorkerPrivate::EventTarget::IsOnCurrentT
   if (!mWorkerPrivate) {
     NS_WARNING("A worker's event target was used after the worker has !");
     return false;
   }
 
   return mWorkerPrivate->IsOnCurrentThread();
 }
 
+WorkerPrivate::AutoPushEventLoopGlobal::AutoPushEventLoopGlobal(
+    WorkerPrivate* aWorkerPrivate, JSContext* aCx)
+    : mWorkerPrivate(aWorkerPrivate) {
+  MOZ_ACCESS_THREAD_BOUND(mWorkerPrivate->mWorkerThreadAccessible, data);
+  mOldEventLoopGlobal = data->mCurrentEventLoopGlobal.forget();
+  if (JSObject* global = JS::CurrentGlobalOrNull(aCx)) {
+    data->mCurrentEventLoopGlobal = xpc::NativeGlobal(global);
+  }
+}
+
+WorkerPrivate::AutoPushEventLoopGlobal::~AutoPushEventLoopGlobal() {
+  MOZ_ACCESS_THREAD_BOUND(mWorkerPrivate->mWorkerThreadAccessible, data);
+  data->mCurrentEventLoopGlobal = mOldEventLoopGlobal.forget();
+}
+
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -312,16 +312,24 @@ class WorkerPrivate : public RelativeTim
     return data->mScope;
   }
 
   WorkerDebuggerGlobalScope* DebuggerGlobalScope() const {
     MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
     return data->mDebuggerScope;
   }
 
+  // Get the global associated with the current nested event loop.  Will return
+  // null if we're not in a nested event loop or that nested event loop does not
+  // have an associated global.
+  nsIGlobalObject* GetCurrentEventLoopGlobal() const {
+    MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
+    return data->mCurrentEventLoopGlobal;
+  }
+
   nsICSPEventListener* CSPEventListener() const;
 
   void SetThread(WorkerThread* aThread);
 
   void SetWorkerPrivateInWorkerThread(WorkerThread* aThread);
 
   void ResetWorkerPrivateInWorkerThread();
 
@@ -1093,31 +1101,55 @@ class WorkerPrivate : public RelativeTim
     nsCOMPtr<nsITimerCallback> mTimerRunnable;
 
     nsCOMPtr<nsITimer> mGCTimer;
 
     RefPtr<MemoryReporter> mMemoryReporter;
 
     UniquePtr<ClientSource> mClientSource;
 
+    // While running a nested event loop, whether a sync loop or a debugger
+    // event loop we want to keep track of which global is running it, if any,
+    // so runnables that run off that event loop can get at that information. In
+    // practice this only matters for various worker debugger runnables running
+    // against sandboxes, because all other runnables know which globals they
+    // belong to already.  We could also address this by threading the relevant
+    // global through the chains of runnables involved, but we'd need to thread
+    // it through some runnables that run on the main thread, and that would
+    // require some care to make sure things get released on the correct thread,
+    // which we'd rather avoid.  This member is only accessed on the worker
+    // thread.
+    nsCOMPtr<nsIGlobalObject> mCurrentEventLoopGlobal;
+
     uint32_t mNumWorkerRefsPreventingShutdownStart;
     uint32_t mDebuggerEventLoopLevel;
 
     uint32_t mErrorHandlerRecursionCount;
     uint32_t mNextTimeoutId;
 
     bool mFrozen;
     bool mTimerRunning;
     bool mRunningExpiredTimeouts;
     bool mPeriodicGCTimerRunning;
     bool mIdleGCTimerRunning;
     bool mOnLine;
   };
   ThreadBound<WorkerThreadAccessible> mWorkerThreadAccessible;
 
+  class MOZ_RAII AutoPushEventLoopGlobal {
+   public:
+    AutoPushEventLoopGlobal(WorkerPrivate* aWorkerPrivate, JSContext* aCx);
+    ~AutoPushEventLoopGlobal();
+
+   private:
+    WorkerPrivate* mWorkerPrivate;
+    nsCOMPtr<nsIGlobalObject> mOldEventLoopGlobal;
+  };
+  friend class AutoPushEventLoopGlobal;
+
   uint32_t mPostSyncLoopOperations;
 
   // List of operations to do at the end of the last sync event loop.
   enum {
     ePendingEventQueueClearing = 0x01,
     eDispatchCancelingRunnable = 0x02,
   };
 
--- a/dom/workers/WorkerRunnable.cpp
+++ b/dom/workers/WorkerRunnable.cpp
@@ -264,25 +264,18 @@ WorkerRunnable::Run() {
   }
 
   // Track down the appropriate global, if any, to use for the AutoEntryScript.
   nsCOMPtr<nsIGlobalObject> globalObject;
   bool isMainThread = !targetIsWorkerThread && !mWorkerPrivate->GetParent();
   MOZ_ASSERT(isMainThread == NS_IsMainThread());
   RefPtr<WorkerPrivate> kungFuDeathGrip;
   if (targetIsWorkerThread) {
-    JSContext* cx = GetCurrentWorkerThreadJSContext();
-    if (NS_WARN_IF(!cx)) {
-      return NS_ERROR_FAILURE;
-    }
-
-    JSObject* global = JS::CurrentGlobalOrNull(cx);
-    if (global) {
-      globalObject = xpc::NativeGlobal(global);
-    } else {
+    globalObject = mWorkerPrivate->GetCurrentEventLoopGlobal();
+    if (!globalObject) {
       globalObject = DefaultGlobalObject();
     }
 
     // We may still not have a globalObject here: in the case of
     // CompileScriptRunnable, we don't actually create the global object until
     // we have the script data, which happens in a syncloop under
     // CompileScriptRunnable::WorkerRun, so we can't assert that it got created
     // in the PreRun call above.