Bug 823953: Improve sync queue handling. r=bent
authorKyle Huey <khuey@kylehuey.com>
Fri, 21 Dec 2012 12:14:47 -0800
changeset 125928 95807c27bf993199c66a9206fa9e5b120001b874
parent 125927 014b47f440c5da47d2c66b19dd30b2b559a2bf5b
child 125929 3a086666f64512479d27215f4a1f174b97a1835b
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs823953
milestone20.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 823953: Improve sync queue handling. r=bent
dom/workers/RuntimeService.cpp
dom/workers/ScriptLoader.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
dom/workers/XMLHttpRequest.cpp
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -301,24 +301,25 @@ public:
     mSyncQueueKey(0)
   {
     NS_ASSERTION(aWorker, "WorkerPrivate cannot be null");
   }
 
   bool
   Dispatch(JSContext* aCx)
   {
-    mSyncQueueKey = mWorkerPrivate->CreateNewSyncLoop();
+    AutoSyncLoopHolder syncLoop(mWorkerPrivate);
+    mSyncQueueKey = syncLoop.SyncQueueKey();
 
     if (NS_FAILED(NS_DispatchToMainThread(this, NS_DISPATCH_NORMAL))) {
       JS_ReportError(aCx, "Failed to dispatch to main thread!");
       return false;
     }
 
-    return mWorkerPrivate->RunSyncLoop(aCx, mSyncQueueKey);
+    return syncLoop.RunAndForget(aCx);
   }
 
   NS_IMETHOD
   Run()
   {
     AssertIsOnMainThread();
 
     nsIContentSecurityPolicy* csp = mWorkerPrivate->GetCSP();
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -661,36 +661,36 @@ ScriptExecutorRunnable::PostRun(JSContex
 
 bool
 LoadAllScripts(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                nsTArray<ScriptLoadInfo>& aLoadInfos, bool aIsWorkerScript)
 {
   aWorkerPrivate->AssertIsOnWorkerThread();
   NS_ASSERTION(!aLoadInfos.IsEmpty(), "Bad arguments!");
 
-  uint32_t syncQueueKey = aWorkerPrivate->CreateNewSyncLoop();
+  AutoSyncLoopHolder syncLoop(aWorkerPrivate);
 
   nsRefPtr<ScriptLoaderRunnable> loader =
-    new ScriptLoaderRunnable(aWorkerPrivate, syncQueueKey, aLoadInfos,
-                             aIsWorkerScript);
+    new ScriptLoaderRunnable(aWorkerPrivate, syncLoop.SyncQueueKey(),
+                             aLoadInfos, aIsWorkerScript);
 
   NS_ASSERTION(aLoadInfos.IsEmpty(), "Should have swapped!");
 
   if (!aWorkerPrivate->AddFeature(aCx, loader)) {
     return false;
   }
 
   if (NS_FAILED(NS_DispatchToMainThread(loader, NS_DISPATCH_NORMAL))) {
     NS_ERROR("Failed to dispatch!");
 
     aWorkerPrivate->RemoveFeature(aCx, loader);
     return false;
   }
 
-  return aWorkerPrivate->RunSyncLoop(aCx, syncQueueKey);
+  return syncLoop.RunAndForget(aCx);
 }
 
 } /* anonymous namespace */
 
 BEGIN_WORKERS_NAMESPACE
 
 namespace scriptloader {
 
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -3385,17 +3385,17 @@ WorkerPrivate::RunSyncLoop(JSContext* aC
 #endif
 
     if (syncQueue->mComplete) {
       NS_ASSERTION(mSyncQueues.Length() - 1 == aSyncLoopKey,
                    "Mismatched calls!");
       NS_ASSERTION(syncQueue->mQueue.IsEmpty(), "Unprocessed sync events!");
 
       bool result = syncQueue->mResult;
-      mSyncQueues.RemoveElementAt(aSyncLoopKey);
+      DestroySyncLoop(aSyncLoopKey);
 
 #ifdef DEBUG
       syncQueue = nullptr;
 #endif
 
       return result;
     }
   }
@@ -3419,16 +3419,24 @@ WorkerPrivate::StopSyncLoop(uint32_t aSy
   SyncQueue* syncQueue = mSyncQueues[aSyncLoopKey].get();
 
   NS_ASSERTION(!syncQueue->mComplete, "Already called StopSyncLoop?!");
 
   syncQueue->mResult = aSyncResult;
   syncQueue->mComplete = true;
 }
 
+void
+WorkerPrivate::DestroySyncLoop(uint32_t aSyncLoopKey)
+{
+  AssertIsOnWorkerThread();
+
+  mSyncQueues.RemoveElementAt(aSyncLoopKey);
+}
+
 bool
 WorkerPrivate::PostMessageToParent(JSContext* aCx, jsval aMessage,
                                    jsval aTransferable)
 {
   AssertIsOnWorkerThread();
 
   JSStructuredCloneCallbacks* callbacks =
     IsChromeWorker() ?
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -656,16 +656,19 @@ public:
   CreateNewSyncLoop();
 
   bool
   RunSyncLoop(JSContext* aCx, uint32_t aSyncLoopKey);
 
   void
   StopSyncLoop(uint32_t aSyncLoopKey, bool aSyncResult);
 
+  void
+  DestroySyncLoop(uint32_t aSyncLoopKey);
+
   bool
   PostMessageToParent(JSContext* aCx, jsval aMessage,
                       jsval transferable);
 
   bool
   NotifyInternal(JSContext* aCx, Status aStatus);
 
   void
@@ -843,11 +846,47 @@ enum WorkerStructuredDataType
 };
 
 JSStructuredCloneCallbacks*
 WorkerStructuredCloneCallbacks(bool aMainRuntime);
 
 JSStructuredCloneCallbacks*
 ChromeWorkerStructuredCloneCallbacks(bool aMainRuntime);
 
+class AutoSyncLoopHolder
+{
+public:
+  AutoSyncLoopHolder(WorkerPrivate* aWorkerPrivate)
+  : mWorkerPrivate(aWorkerPrivate), mSyncLoopKey(UINT32_MAX)
+  {
+    mSyncLoopKey = mWorkerPrivate->CreateNewSyncLoop();
+  }
+
+  ~AutoSyncLoopHolder()
+  {
+    if (mWorkerPrivate) {
+      mWorkerPrivate->StopSyncLoop(mSyncLoopKey, false);
+      mWorkerPrivate->DestroySyncLoop(mSyncLoopKey);
+    }
+  }
+
+  bool
+  RunAndForget(JSContext* aCx)
+  {
+    WorkerPrivate* workerPrivate = mWorkerPrivate;
+    mWorkerPrivate = nullptr;
+    return workerPrivate->RunSyncLoop(aCx, mSyncLoopKey);
+  }
+
+  uint32_t
+  SyncQueueKey() const
+  {
+    return mSyncLoopKey;
+  }
+
+private:
+  WorkerPrivate* mWorkerPrivate;
+  uint32_t mSyncLoopKey;
+};
+
 END_WORKERS_NAMESPACE
 
 #endif /* mozilla_dom_workers_workerprivate_h__ */
--- a/dom/workers/XMLHttpRequest.cpp
+++ b/dom/workers/XMLHttpRequest.cpp
@@ -825,28 +825,25 @@ public:
     NS_ASSERTION(aProxy, "Don't hand me a null proxy!");
   }
 
   bool
   Dispatch(JSContext* aCx)
   {
     mWorkerPrivate->AssertIsOnWorkerThread();
 
-    mSyncQueueKey = mWorkerPrivate->CreateNewSyncLoop();
+    AutoSyncLoopHolder syncLoop(mWorkerPrivate);
+    mSyncQueueKey = syncLoop.SyncQueueKey();
 
     if (NS_FAILED(NS_DispatchToMainThread(this, NS_DISPATCH_NORMAL))) {
       JS_ReportError(aCx, "Failed to dispatch to main thread!");
       return false;
     }
 
-    if (!mWorkerPrivate->RunSyncLoop(aCx, mSyncQueueKey)) {
-      return false;
-    }
-
-    return true;
+    return syncLoop.RunAndForget(aCx);
   }
 
   virtual nsresult
   MainThreadRun() = 0;
 
   NS_IMETHOD
   Run()
   {
@@ -1702,43 +1699,54 @@ XMLHttpRequest::SendInternal(const nsASt
   bool hasUploadListeners = mUpload ? mUpload->HasListeners() : false;
 
   MaybePin(aRv);
   if (aRv.Failed()) {
     return;
   }
 
   AutoUnpinXHR autoUnpin(this);
+  Maybe<AutoSyncLoopHolder> autoSyncLoop;
 
   uint32_t syncQueueKey = UINT32_MAX;
-  if (mProxy->mIsSyncXHR) {
-    syncQueueKey = mWorkerPrivate->CreateNewSyncLoop();
+  bool isSyncXHR = mProxy->mIsSyncXHR;
+  if (isSyncXHR) {
+    autoSyncLoop.construct(mWorkerPrivate);
+    syncQueueKey = autoSyncLoop.ref().SyncQueueKey();
   }
 
   mProxy->mOuterChannelId++;
 
   JSContext* cx = GetJSContext();
 
   nsRefPtr<SendRunnable> runnable =
     new SendRunnable(mWorkerPrivate, mProxy, aStringBody, aBody,
                      aClonedObjects, syncQueueKey, hasUploadListeners);
   if (!runnable->Dispatch(cx)) {
     return;
   }
 
-  autoUnpin.Clear();
+  if (!isSyncXHR)  {
+    autoUnpin.Clear();
+    MOZ_ASSERT(autoSyncLoop.empty());
+    return;
+  }
 
-  // The event loop was spun above, make sure we aren't canceled already.
+  // If our sync XHR was canceled during the send call the worker is going
+  // away.  We have no idea how far through the send call we got.  There may
+  // be a ProxyCompleteRunnable in the sync loop, but rather than run the loop
+  // to get it we just let our RAII helpers clean up.
   if (mCanceled) {
     return;
   }
 
-  if (mProxy->mIsSyncXHR && !mWorkerPrivate->RunSyncLoop(cx, syncQueueKey)) {
+  autoUnpin.Clear();
+
+  if (!autoSyncLoop.ref().RunAndForget(cx)) {
     aRv.Throw(NS_ERROR_FAILURE);
-    return;
   }
 }
 
 bool
 XMLHttpRequest::Notify(JSContext* aCx, Status aStatus)
 {
   mWorkerPrivate->AssertIsOnWorkerThread();
   MOZ_ASSERT(GetJSContext() == aCx);