Bug 1399646: Part 2 - Use the async shutdown service for ServiceWorkerRegistrar. r?baku draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 13 Sep 2017 15:13:38 -0700
changeset 664368 9322440ae678a854f23acaaedeeb2bb98705e247
parent 664367 7e5f619f8eed0e0ce3f1b94e8285d12a4be29d51
child 664369 8c04e4be2f74850f28d642350b9ff268ab3206e4
push id79689
push usermaglione.k@gmail.com
push dateWed, 13 Sep 2017 22:25:50 +0000
reviewersbaku
bugs1399646
milestone57.0a1
Bug 1399646: Part 2 - Use the async shutdown service for ServiceWorkerRegistrar. r?baku The current shutdown handling code is susceptible to deadlocks, since it spins the event loop while it holds mMonitor, and other main thread methods which try to acquire mMonitor can be called from code that runs while the event loop is spinning. My initial solution was just to release mMonitor before spinning the event loop, but at this point I think it makes more sense to switch to the standardized AsyncShutdown routines, which provide better diagnostics and allow us to avoid one more nested event loop during shutdown. MozReview-Commit-ID: 1RtFN585IR7
dom/workers/ServiceWorkerRegistrar.cpp
dom/workers/ServiceWorkerRegistrar.h
--- a/dom/workers/ServiceWorkerRegistrar.cpp
+++ b/dom/workers/ServiceWorkerRegistrar.cpp
@@ -49,17 +49,18 @@ static const char* gSupportedRegistrarVe
   "2"
 };
 
 StaticRefPtr<ServiceWorkerRegistrar> gServiceWorkerRegistrar;
 
 } // namespace
 
 NS_IMPL_ISUPPORTS(ServiceWorkerRegistrar,
-                  nsIObserver)
+                  nsIObserver,
+                  nsIAsyncShutdownBlocker)
 
 void
 ServiceWorkerRegistrar::Initialize()
 {
   MOZ_ASSERT(!gServiceWorkerRegistrar);
 
   if (!XRE_IsParentProcess()) {
     return;
@@ -68,20 +69,16 @@ ServiceWorkerRegistrar::Initialize()
   gServiceWorkerRegistrar = new ServiceWorkerRegistrar();
   ClearOnShutdown(&gServiceWorkerRegistrar);
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     DebugOnly<nsresult> rv = obs->AddObserver(gServiceWorkerRegistrar,
                                               "profile-after-change", false);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
-
-    rv = obs->AddObserver(gServiceWorkerRegistrar, "profile-before-change",
-                          false);
-    MOZ_ASSERT(NS_SUCCEEDED(rv));
   }
 }
 
 /* static */ already_AddRefed<ServiceWorkerRegistrar>
 ServiceWorkerRegistrar::Get()
 {
   MOZ_ASSERT(XRE_IsParentProcess());
 
@@ -89,17 +86,16 @@ ServiceWorkerRegistrar::Get()
   RefPtr<ServiceWorkerRegistrar> service = gServiceWorkerRegistrar.get();
   return service.forget();
 }
 
 ServiceWorkerRegistrar::ServiceWorkerRegistrar()
   : mMonitor("ServiceWorkerRegistrar.mMonitor")
   , mDataLoaded(false)
   , mShuttingDown(false)
-  , mShutdownCompleteFlag(nullptr)
   , mRunnableCounter(0)
 {
   MOZ_ASSERT(NS_IsMainThread());
 }
 
 ServiceWorkerRegistrar::~ServiceWorkerRegistrar()
 {
   MOZ_ASSERT(!mRunnableCounter);
@@ -824,18 +820,18 @@ ServiceWorkerRegistrar::ScheduleSaveData
   ++mRunnableCounter;
 }
 
 void
 ServiceWorkerRegistrar::ShutdownCompleted()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  MOZ_ASSERT(mShutdownCompleteFlag && !*mShutdownCompleteFlag);
-  *mShutdownCompleteFlag = true;
+  DebugOnly<nsresult> rv = GetShutdownPhase()->RemoveBlocker(this);
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
 }
 
 void
 ServiceWorkerRegistrar::SaveData()
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
   nsresult rv = WriteData();
@@ -1022,16 +1018,21 @@ ServiceWorkerRegistrar::ProfileStarted()
   MOZ_DIAGNOSTIC_ASSERT(!mProfileDir);
 
   nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
                                        getter_AddRefs(mProfileDir));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
+  rv = GetShutdownPhase()->AddBlocker(
+    this, NS_LITERAL_STRING(__FILE__), __LINE__,
+    NS_LITERAL_STRING("ServiceWorkerRegistrar"));
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
+
   nsCOMPtr<nsIEventTarget> target =
     do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID);
   MOZ_ASSERT(target, "Must have stream transport service");
 
   nsCOMPtr<nsIRunnable> runnable =
     NewRunnableMethod("dom::ServiceWorkerRegistrar::LoadData",
                       this,
                       &ServiceWorkerRegistrar::LoadData);
@@ -1056,22 +1057,51 @@ ServiceWorkerRegistrar::ProfileStopped()
     }
   }
 
   PBackgroundChild* child = BackgroundChild::GetForCurrentThread();
   if (!child) {
     return;
   }
 
-  bool completed = false;
-  mShutdownCompleteFlag = &completed;
+  child->SendShutdownServiceWorkerRegistrar();
+}
+
+// Async shutdown blocker methods
+
+NS_IMETHODIMP
+ServiceWorkerRegistrar::BlockShutdown(nsIAsyncShutdownClient* aClient)
+{
+  ProfileStopped();
+  return NS_OK;
+}
 
-  child->SendShutdownServiceWorkerRegistrar();
+NS_IMETHODIMP
+ServiceWorkerRegistrar::GetName(nsAString& aName)
+{
+  aName = NS_LITERAL_STRING("ServiceWorkerRegistar");
+  return NS_OK;
+}
 
-  MOZ_ALWAYS_TRUE(SpinEventLoopUntil([&]() { return completed; }));
+NS_IMETHODIMP
+ServiceWorkerRegistrar::GetState(nsIPropertyBag**)
+{
+  return NS_OK;
+}
+
+nsCOMPtr<nsIAsyncShutdownClient>
+ServiceWorkerRegistrar::GetShutdownPhase() const
+{
+  nsCOMPtr<nsIAsyncShutdownService> svc = services::GetAsyncShutdown();
+  MOZ_RELEASE_ASSERT(svc);
+
+  nsCOMPtr<nsIAsyncShutdownClient> client;
+  Unused << svc->GetProfileBeforeChange(getter_AddRefs(client));
+  MOZ_RELEASE_ASSERT(client);
+  return Move(client);
 }
 
 void
 ServiceWorkerRegistrar::Shutdown()
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(!mShuttingDown);
 
@@ -1092,25 +1122,14 @@ ServiceWorkerRegistrar::Observe(nsISuppo
 
     // The profile is fully loaded, now we can proceed with the loading of data
     // from disk.
     ProfileStarted();
 
     return NS_OK;
   }
 
-  if (!strcmp(aTopic, "profile-before-change")) {
-    nsCOMPtr<nsIObserverService> observerService =
-      services::GetObserverService();
-    observerService->RemoveObserver(this, "profile-before-change");
-
-    // Shutting down, let's sync the data.
-    ProfileStopped();
-
-    return NS_OK;
-  }
-
   MOZ_ASSERT(false, "ServiceWorkerRegistrar got unexpected topic!");
   return NS_ERROR_UNEXPECTED;
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/workers/ServiceWorkerRegistrar.h
+++ b/dom/workers/ServiceWorkerRegistrar.h
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_workers_ServiceWorkerRegistrar_h
 #define mozilla_dom_workers_ServiceWorkerRegistrar_h
 
 #include "mozilla/Monitor.h"
 #include "mozilla/Telemetry.h"
 #include "nsClassHashtable.h"
+#include "nsIAsyncShutdown.h"
 #include "nsIObserver.h"
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "nsTArray.h"
 
 #define SERVICEWORKERREGISTRAR_FILE "serviceworker.txt"
 #define SERVICEWORKERREGISTRAR_VERSION "8"
 #define SERVICEWORKERREGISTRAR_TERMINATOR "#"
@@ -29,22 +30,24 @@ namespace ipc {
 class PrincipalInfo;
 } // namespace ipc
 
 namespace dom {
 
 class ServiceWorkerRegistrationData;
 
 class ServiceWorkerRegistrar : public nsIObserver
+                             , public nsIAsyncShutdownBlocker
 {
   friend class ServiceWorkerRegistrarSaveDataRunnable;
 
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIOBSERVER
+  NS_DECL_NSIASYNCSHUTDOWNBLOCKER
 
   static void Initialize();
 
   void Shutdown();
 
   void DataSaved();
 
   static already_AddRefed<ServiceWorkerRegistrar> Get();
@@ -74,28 +77,29 @@ protected:
 private:
   void ProfileStarted();
   void ProfileStopped();
 
   void ScheduleSaveData();
   void ShutdownCompleted();
   void MaybeScheduleShutdownCompleted();
 
+  nsCOMPtr<nsIAsyncShutdownClient> GetShutdownPhase() const;
+
   bool IsSupportedVersion(const nsACString& aVersion) const;
 
   mozilla::Monitor mMonitor;
 
 protected:
   // protected by mMonitor.
   nsCOMPtr<nsIFile> mProfileDir;
   nsTArray<ServiceWorkerRegistrationData> mData;
   bool mDataLoaded;
 
   // PBackground thread only
   bool mShuttingDown;
-  bool* mShutdownCompleteFlag;
   uint32_t mRunnableCounter;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_workers_ServiceWorkerRegistrar_h