Bug 1457157 P2 Clear a worker's ClientSource when it reaches Terminating. r=baku
authorBen Kelly <ben@wanderview.com>
Wed, 02 May 2018 06:29:26 -0700
changeset 416705 c86679a9bf8360dd95790c63768a70913467c46a
parent 416704 d667f861e2d11efa1fa2e6fd135bddc5b1900e03
child 416706 9184164d3c6c714b148f6ba36df3b85f6a9b5f1b
push id33933
push userrgurzau@mozilla.com
push dateWed, 02 May 2018 21:05:42 +0000
treeherdermozilla-central@2d83e1843241 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1457157
milestone61.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 1457157 P2 Clear a worker's ClientSource when it reaches Terminating. r=baku
dom/fetch/Fetch.cpp
dom/workers/ScriptLoader.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
dom/workers/WorkerScope.cpp
dom/xhr/XMLHttpRequestWorker.cpp
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -530,18 +530,24 @@ FetchRequest(nsIGlobalObject* aGlobal, c
     RefPtr<WorkerFetchResolver> resolver =
       WorkerFetchResolver::Create(worker, p, signal, observer);
     if (!resolver) {
       NS_WARNING("Could not keep the worker alive.");
       aRv.Throw(NS_ERROR_DOM_ABORT_ERR);
       return nullptr;
     }
 
+    Maybe<ClientInfo> clientInfo(worker->GetClientInfo());
+    if (clientInfo.isNothing()) {
+      aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+      return nullptr;
+    }
+
     RefPtr<MainThreadFetchRunnable> run =
-      new MainThreadFetchRunnable(resolver, worker->GetClientInfo(),
+      new MainThreadFetchRunnable(resolver, clientInfo.ref(),
                                   worker->GetController(), r);
     worker->DispatchToMainThread(run.forget());
   }
 
   return p.forget();
 }
 
 void
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -1905,17 +1905,19 @@ class ChannelGetterRunnable final : publ
 
 public:
   ChannelGetterRunnable(WorkerPrivate* aParentWorker,
                         const nsAString& aScriptURL,
                         WorkerLoadInfo& aLoadInfo)
     : WorkerMainThreadRunnable(aParentWorker,
                                NS_LITERAL_CSTRING("ScriptLoader :: ChannelGetter"))
     , mScriptURL(aScriptURL)
-    , mClientInfo(aParentWorker->GetClientInfo())
+    // ClientInfo should always be present since this should not be called
+    // if parent's status is greater than Running.
+    , mClientInfo(aParentWorker->GetClientInfo().ref())
     , mLoadInfo(aLoadInfo)
     , mResult(NS_ERROR_FAILURE)
   {
     MOZ_ASSERT(aParentWorker);
     aParentWorker->AssertIsOnWorkerThread();
   }
 
   virtual bool
@@ -2241,17 +2243,17 @@ LoadAllScripts(WorkerPrivate* aWorkerPri
   if (!syncLoopTarget) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
   Maybe<ClientInfo> clientInfo;
   Maybe<ServiceWorkerDescriptor> controller;
   if (!aIsMainScript) {
-    clientInfo.emplace(aWorkerPrivate->GetClientInfo());
+    clientInfo = aWorkerPrivate->GetClientInfo();
     controller = aWorkerPrivate->GetController();
   }
 
   RefPtr<ScriptLoaderRunnable> loader =
     new ScriptLoaderRunnable(aWorkerPrivate, syncLoopTarget, aLoadInfos,
                              clientInfo, controller,
                              aIsMainScript, aWorkerScriptType, aRv);
 
@@ -2385,17 +2387,17 @@ LoadMainScript(WorkerPrivate* aWorkerPri
   nsTArray<ScriptLoadInfo> loadInfos;
 
   ScriptLoadInfo* info = loadInfos.AppendElement();
   info->mURL = aScriptURL;
   info->mLoadFlags = aWorkerPrivate->GetLoadFlags();
 
   // We are loading the main script, so the worker's Client must be
   // reserved.
-  info->mReservedClientInfo.emplace(aWorkerPrivate->GetClientInfo());
+  info->mReservedClientInfo = aWorkerPrivate->GetClientInfo();
 
   LoadAllScripts(aWorkerPrivate, loadInfos, true, aWorkerScriptType, aRv);
 }
 
 void
 Load(WorkerPrivate* aWorkerPrivate,
      const nsTArray<nsString>& aScriptURLs, WorkerScriptType aWorkerScriptType,
      ErrorResult& aRv)
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -3273,22 +3273,24 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
         MOZ_ASSERT(currentStatus == Killing);
 #else
         currentStatus = Killing;
 #endif
       }
 
       // If we're supposed to die then we should exit the loop.
       if (currentStatus == Killing) {
+        // The ClientSource should be cleared in NotifyInternal() when we reach
+        // or pass Terminating.
+        MOZ_DIAGNOSTIC_ASSERT(!mClientSource);
+
         // Flush uncaught rejections immediately, without
         // waiting for a next tick.
         PromiseDebugging::FlushUncaughtRejections();
 
-        mClientSource = nullptr;
-
         ShutdownGCTimers();
 
         DisableMemoryReporter();
 
         {
           MutexAutoLock lock(mMutex);
 
           mStatus = Dead;
@@ -3495,22 +3497,27 @@ WorkerPrivate::EnsurePerformanceStorage(
 {
   AssertIsOnWorkerThread();
 
   if (!mPerformanceStorage) {
     mPerformanceStorage = PerformanceStorageWorker::Create(this);
   }
 }
 
-const ClientInfo&
+Maybe<ClientInfo>
 WorkerPrivate::GetClientInfo() const
 {
   AssertIsOnWorkerThread();
-  MOZ_DIAGNOSTIC_ASSERT(mClientSource);
-  return mClientSource->Info();
+  Maybe<ClientInfo> clientInfo;
+  if (!mClientSource) {
+    MOZ_DIAGNOSTIC_ASSERT(mStatus >= Terminating);
+    return Move(clientInfo);
+  }
+  clientInfo.emplace(mClientSource->Info());
+  return Move(clientInfo);
 }
 
 const ClientState
 WorkerPrivate::GetClientState() const
 {
   AssertIsOnWorkerThread();
   MOZ_DIAGNOSTIC_ASSERT(mClientSource);
   ClientState state;
@@ -3535,16 +3542,22 @@ WorkerPrivate::Control(const ServiceWork
   MOZ_DIAGNOSTIC_ASSERT(Type() != WorkerTypeService);
   mClientSource->SetController(aServiceWorker);
 }
 
 void
 WorkerPrivate::ExecutionReady()
 {
   AssertIsOnWorkerThread();
+  {
+    MutexAutoLock lock(mMutex);
+    if (mStatus >= Terminating) {
+      return;
+    }
+  }
   MOZ_DIAGNOSTIC_ASSERT(mClientSource);
   mClientSource->WorkerExecutionReady(this);
 }
 
 void
 WorkerPrivate::InitializeGCTimers()
 {
   AssertIsOnWorkerThread();
@@ -4509,18 +4522,19 @@ WorkerPrivate::NotifyInternal(WorkerStat
   {
     MutexAutoLock lock(mMutex);
 
     if (mStatus >= aStatus) {
       return true;
     }
 
     if (aStatus >= Terminating) {
+      MutexAutoUnlock unlock(mMutex);
+      mClientSource.reset();
       if (mScope) {
-        MutexAutoUnlock unlock(mMutex);
         mScope->NoteTerminating();
       }
     }
 
     // Make sure the hybrid event target stops dispatching runnables
     // once we reaching the killing state.
     if (aStatus == Killing) {
       // To avoid deadlock we always acquire the event target mutex before the
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -548,17 +548,17 @@ public:
   EnsureClientSource();
 
   void
   EnsurePerformanceStorage();
 
   void
   EnsurePerformanceCounter();
 
-  const ClientInfo&
+  Maybe<ClientInfo>
   GetClientInfo() const;
 
   const ClientState
   GetClientState() const;
 
   const Maybe<ServiceWorkerDescriptor>
   GetController() const;
 
--- a/dom/workers/WorkerScope.cpp
+++ b/dom/workers/WorkerScope.cpp
@@ -541,19 +541,17 @@ AbstractThread*
 WorkerGlobalScope::AbstractMainThreadFor(TaskCategory aCategory)
 {
   MOZ_CRASH("AbstractMainThreadFor not supported for workers.");
 }
 
 Maybe<ClientInfo>
 WorkerGlobalScope::GetClientInfo() const
 {
-  Maybe<ClientInfo> info;
-  info.emplace(mWorkerPrivate->GetClientInfo());
-  return Move(info);
+  return mWorkerPrivate->GetClientInfo();
 }
 
 Maybe<ClientState>
 WorkerGlobalScope::GetClientState() const
 {
   Maybe<ClientState> state;
   state.emplace(mWorkerPrivate->GetClientState());
   return Move(state);
--- a/dom/xhr/XMLHttpRequestWorker.cpp
+++ b/dom/xhr/XMLHttpRequestWorker.cpp
@@ -1870,17 +1870,22 @@ XMLHttpRequestWorker::Open(const nsACStr
 
   if (mProxy) {
     MaybeDispatchPrematureAbortEvents(aRv);
     if (aRv.Failed()) {
       return;
     }
   }
   else {
-    mProxy = new Proxy(this, mWorkerPrivate->GetClientInfo(),
+    Maybe<ClientInfo> clientInfo(mWorkerPrivate->GetClientInfo());
+    if (clientInfo.isNothing()) {
+      aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+      return;
+    }
+    mProxy = new Proxy(this, clientInfo.ref(),
                        mWorkerPrivate->GetController(), mMozAnon, mMozSystem);
   }
 
   mProxy->mOuterEventStreamId++;
 
   RefPtr<OpenRunnable> runnable =
     new OpenRunnable(mWorkerPrivate, mProxy, aMethod, aUrl, aUser, aPassword,
                      mBackgroundRequest, mWithCredentials,