Bug 1350637 - Part 6: Fix a deadlock when main process storage child actor triggers storage thread initialization; r=asuth
authorJan Varga <jan.varga@gmail.com>
Tue, 08 Aug 2017 23:01:52 +0200
changeset 373545 371e75eb2b60281508ef8f7535bdda3964975921
parent 373544 a781f14e975bb4e58c38dc5e86a216647f3a61d7
child 373546 28506433e4f03f5c1d2f42fc59c4f86af6da3181
push id32304
push usercbook@mozilla.com
push dateWed, 09 Aug 2017 09:37:21 +0000
treeherdermozilla-central@4c5fbf493763 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1350637
milestone57.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 1350637 - Part 6: Fix a deadlock when main process storage child actor triggers storage thread initialization; r=asuth
dom/storage/StorageDBThread.cpp
dom/storage/StorageDBThread.h
dom/storage/StorageIPC.cpp
dom/storage/StorageIPC.h
ipc/glue/BackgroundChildImpl.cpp
ipc/glue/BackgroundChildImpl.h
ipc/glue/BackgroundParentImpl.cpp
ipc/glue/BackgroundParentImpl.h
ipc/glue/PBackground.ipdl
--- a/dom/storage/StorageDBThread.cpp
+++ b/dom/storage/StorageDBThread.cpp
@@ -192,50 +192,88 @@ StorageDBThread::Get()
 {
   AssertIsOnBackgroundThread();
 
   return sStorageThread;
 }
 
 // static
 StorageDBThread*
-StorageDBThread::GetOrCreate()
+StorageDBThread::GetOrCreate(const nsString& aProfilePath)
 {
   AssertIsOnBackgroundThread();
 
   if (sStorageThread || sStorageThreadDown) {
     // When sStorageThreadDown is at true, sStorageThread is null.
     // Checking sStorageThreadDown flag here prevents reinitialization of
     // the storage thread after shutdown.
     return sStorageThread;
   }
 
   nsAutoPtr<StorageDBThread> storageThread(new StorageDBThread());
 
-  nsresult rv = storageThread->Init();
+  nsresult rv = storageThread->Init(aProfilePath);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
 
   sStorageThread = storageThread.forget();
 
   return sStorageThread;
 }
 
+// static
 nsresult
-StorageDBThread::Init()
+StorageDBThread::GetProfilePath(nsString& aProfilePath)
+{
+  MOZ_ASSERT(XRE_IsParentProcess());
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // Need to determine location on the main thread, since
+  // NS_GetSpecialDirectory accesses the atom table that can
+  // only be accessed on the main thread.
+  nsCOMPtr<nsIFile> profileDir;
+  nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
+                                       getter_AddRefs(profileDir));
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  rv = profileDir->GetPath(aProfilePath);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  // This service has to be started on the main thread currently.
+  nsCOMPtr<mozIStorageService> ss =
+    do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID, &rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  return NS_OK;
+}
+
+nsresult
+StorageDBThread::Init(const nsString& aProfilePath)
 {
   AssertIsOnBackgroundThread();
 
-  RefPtr<InitHelper> helper = new InitHelper();
+  nsresult rv;
 
   nsString profilePath;
-  nsresult rv = helper->SyncDispatchAndReturnProfilePath(profilePath);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+  if (aProfilePath.IsEmpty()) {
+    RefPtr<InitHelper> helper = new InitHelper();
+
+    rv = helper->SyncDispatchAndReturnProfilePath(profilePath);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+  } else {
+    profilePath = aProfilePath;
   }
 
   mDatabaseFile = do_CreateInstance(NS_LOCAL_FILE_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   rv = mDatabaseFile->InitWithPath(profilePath);
@@ -1617,54 +1655,23 @@ InitHelper::SyncDispatchAndReturnProfile
   if (NS_WARN_IF(NS_FAILED(mMainThreadResultCode))) {
     return mMainThreadResultCode;
   }
 
   aProfilePath = mProfilePath;
   return NS_OK;
 }
 
-nsresult
-StorageDBThread::
-InitHelper::RunOnMainThread()
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  // Need to determine location on the main thread, since
-  // NS_GetSpecialDirectory accesses the atom table that can
-  // only be accessed on the main thread.
-  nsCOMPtr<nsIFile> profileDir;
-  nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
-                                       getter_AddRefs(profileDir));
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  rv = profileDir->GetPath(mProfilePath);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  // This service has to be started on the main thread currently.
-  nsCOMPtr<mozIStorageService> ss =
-    do_GetService(MOZ_STORAGE_SERVICE_CONTRACTID, &rv);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 StorageDBThread::
 InitHelper::Run()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  nsresult rv = RunOnMainThread();
+  nsresult rv = GetProfilePath(mProfilePath);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     mMainThreadResultCode = rv;
   }
 
   mozilla::MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mWaiting);
 
   mWaiting = false;
--- a/dom/storage/StorageDBThread.h
+++ b/dom/storage/StorageDBThread.h
@@ -321,19 +321,22 @@ public:
 public:
   StorageDBThread();
   virtual ~StorageDBThread() {}
 
   static StorageDBThread*
   Get();
 
   static StorageDBThread*
-  GetOrCreate();
+  GetOrCreate(const nsString& aProfilePath);
 
-  virtual nsresult Init();
+  static nsresult
+  GetProfilePath(nsString& aProfilePath);
+
+  virtual nsresult Init(const nsString& aProfilePath);
 
   // Flushes all uncommited data and stops the I/O thread.
   virtual nsresult Shutdown();
 
   virtual void AsyncPreload(LocalStorageCacheBridge* aCache,
                             bool aPriority = false)
   {
     InsertDBOp(new DBOperation(aPriority
--- a/dom/storage/StorageIPC.cpp
+++ b/dom/storage/StorageIPC.cpp
@@ -151,19 +151,27 @@ StorageDBChild::Init()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   PBackgroundChild* actor = BackgroundChild::GetOrCreateForCurrentThread();
   if (NS_WARN_IF(!actor)) {
     return NS_ERROR_FAILURE;
   }
 
+  nsString profilePath;
+  if (XRE_IsParentProcess()) {
+    nsresult rv = StorageDBThread::GetProfilePath(profilePath);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+  }
+
   AddIPDLReference();
 
-  actor->SendPBackgroundStorageConstructor(this);
+  actor->SendPBackgroundStorageConstructor(this, profilePath);
 
   nsCOMPtr<nsIObserverService> observerService = services::GetObserverService();
   MOZ_ASSERT(observerService);
 
   nsCOMPtr<nsIObserver> observer = new ShutdownObserver();
 
   MOZ_ALWAYS_SUCCEEDS(
     observerService->AddObserver(observer,
@@ -521,18 +529,19 @@ private:
     return NS_OK;
   }
 
   RefPtr<StorageDBParent> mParent;
 };
 
 } // namespace
 
-StorageDBParent::StorageDBParent()
-: mIPCOpen(false)
+StorageDBParent::StorageDBParent(const nsString& aProfilePath)
+  : mProfilePath(aProfilePath)
+  , mIPCOpen(false)
 {
   AssertIsOnBackgroundThread();
 
   // We are always open by IPC only
   AddIPDLReference();
 
   // Cannot send directly from here since the channel
   // is not completely built at this moment.
@@ -578,31 +587,31 @@ StorageDBParent::ActorDestroy(ActorDestr
   // Implement me! Bug 1005169
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvAsyncPreload(const nsCString& aOriginSuffix,
                                   const nsCString& aOriginNoSuffix,
                                   const bool& aPriority)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   storageThread->AsyncPreload(NewCache(aOriginSuffix, aOriginNoSuffix),
                               aPriority);
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvAsyncGetUsage(const nsCString& aOriginNoSuffix)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   // The object releases it self in LoadUsage method
   RefPtr<UsageParentBridge> usage =
     new UsageParentBridge(this, aOriginNoSuffix);
 
@@ -693,17 +702,17 @@ private:
 mozilla::ipc::IPCResult
 StorageDBParent::RecvPreload(const nsCString& aOriginSuffix,
                              const nsCString& aOriginNoSuffix,
                              const uint32_t& aAlreadyLoadedCount,
                              InfallibleTArray<nsString>* aKeys,
                              InfallibleTArray<nsString>* aValues,
                              nsresult* aRv)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   RefPtr<SyncLoadCacheHelper> cache(
     new SyncLoadCacheHelper(aOriginSuffix, aOriginNoSuffix, aAlreadyLoadedCount,
                             aKeys, aValues, aRv));
 
@@ -713,17 +722,17 @@ StorageDBParent::RecvPreload(const nsCSt
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvAsyncAddItem(const nsCString& aOriginSuffix,
                                   const nsCString& aOriginNoSuffix,
                                   const nsString& aKey,
                                   const nsString& aValue)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   nsresult rv =
     storageThread->AsyncAddItem(NewCache(aOriginSuffix, aOriginNoSuffix),
                                 aKey,
                                 aValue);
@@ -735,17 +744,17 @@ StorageDBParent::RecvAsyncAddItem(const 
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvAsyncUpdateItem(const nsCString& aOriginSuffix,
                                      const nsCString& aOriginNoSuffix,
                                      const nsString& aKey,
                                      const nsString& aValue)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   nsresult rv =
     storageThread->AsyncUpdateItem(NewCache(aOriginSuffix, aOriginNoSuffix),
                                    aKey,
                                    aValue);
@@ -756,17 +765,17 @@ StorageDBParent::RecvAsyncUpdateItem(con
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvAsyncRemoveItem(const nsCString& aOriginSuffix,
                                      const nsCString& aOriginNoSuffix,
                                      const nsString& aKey)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   nsresult rv =
     storageThread->AsyncRemoveItem(NewCache(aOriginSuffix, aOriginNoSuffix),
                                    aKey);
   if (NS_FAILED(rv) && mIPCOpen) {
@@ -775,17 +784,17 @@ StorageDBParent::RecvAsyncRemoveItem(con
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvAsyncClear(const nsCString& aOriginSuffix,
                                 const nsCString& aOriginNoSuffix)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   nsresult rv =
     storageThread->AsyncClear(NewCache(aOriginSuffix, aOriginNoSuffix));
   if (NS_FAILED(rv) && mIPCOpen) {
     mozilla::Unused << SendError(rv);
@@ -805,55 +814,55 @@ StorageDBParent::RecvAsyncFlush()
   storageThread->AsyncFlush();
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvStartup()
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvClearAll()
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   storageThread->AsyncClearAll();
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvClearMatchingOrigin(const nsCString& aOriginNoSuffix)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   storageThread->AsyncClearMatchingOrigin(aOriginNoSuffix);
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 StorageDBParent::RecvClearMatchingOriginAttributes(
                                         const OriginAttributesPattern& aPattern)
 {
-  StorageDBThread* storageThread = StorageDBThread::GetOrCreate();
+  StorageDBThread* storageThread = StorageDBThread::GetOrCreate(mProfilePath);
   if (!storageThread) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   storageThread->AsyncClearMatchingOriginAttributes(aPattern);
 
   return IPC_OK();
 }
@@ -1199,25 +1208,26 @@ ObserverSink::Observe(const char* aTopic
   return NS_OK;
 }
 
 /*******************************************************************************
  * Exported functions
  ******************************************************************************/
 
 PBackgroundStorageParent*
-AllocPBackgroundStorageParent()
+AllocPBackgroundStorageParent(const nsString& aProfilePath)
 {
   AssertIsOnBackgroundThread();
 
-  return new StorageDBParent();
+  return new StorageDBParent(aProfilePath);
 }
 
 mozilla::ipc::IPCResult
-RecvPBackgroundStorageConstructor(PBackgroundStorageParent* aActor)
+RecvPBackgroundStorageConstructor(PBackgroundStorageParent* aActor,
+                                  const nsString& aProfilePath)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aActor);
 
   return IPC_OK();
 }
 
 bool
--- a/dom/storage/StorageIPC.h
+++ b/dom/storage/StorageIPC.h
@@ -138,17 +138,17 @@ private:
 // such as cookie cleaning etc to the child process.
 class StorageDBParent final : public PBackgroundStorageParent
 {
   class ObserverSink;
 
   virtual ~StorageDBParent();
 
 public:
-  StorageDBParent();
+  explicit StorageDBParent(const nsString& aProfilePath);
 
   void
   Init();
 
   NS_IMETHOD_(MozExternalRefCountType) AddRef(void);
   NS_IMETHOD_(MozExternalRefCountType) Release(void);
 
   void AddIPDLReference();
@@ -266,28 +266,36 @@ private:
                const nsCString& aOriginScope);
 
 private:
   CacheParentBridge* NewCache(const nsACString& aOriginSuffix,
                               const nsACString& aOriginNoSuffix);
 
   RefPtr<ObserverSink> mObserverSink;
 
+  // A hack to deal with deadlock between the parent process main thread and
+  // background thread when invoking StorageDBThread::GetOrCreate because it
+  // cannot safely perform a synchronous dispatch back to the main thread
+  // (because we are already synchronously doing things on the stack).
+  // Populated for the same process actors, empty for other process actors.
+  nsString mProfilePath;
+
   ThreadSafeAutoRefCnt mRefCnt;
   NS_DECL_OWNINGTHREAD
 
   // True when IPC channel is open and Send*() methods are OK to use.
   bool mIPCOpen;
 };
 
 PBackgroundStorageParent*
-AllocPBackgroundStorageParent();
+AllocPBackgroundStorageParent(const nsString& aProfilePath);
 
 mozilla::ipc::IPCResult
-RecvPBackgroundStorageConstructor(PBackgroundStorageParent* aActor);
+RecvPBackgroundStorageConstructor(PBackgroundStorageParent* aActor,
+                                  const nsString& aProfilePath);
 
 bool
 DeallocPBackgroundStorageParent(PBackgroundStorageParent* aActor);
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_StorageIPC_h
--- a/ipc/glue/BackgroundChildImpl.cpp
+++ b/ipc/glue/BackgroundChildImpl.cpp
@@ -200,17 +200,17 @@ BackgroundChildImpl::DeallocPBackgroundI
 {
   MOZ_ASSERT(aActor);
 
   delete aActor;
   return true;
 }
 
 BackgroundChildImpl::PBackgroundStorageChild*
-BackgroundChildImpl::AllocPBackgroundStorageChild()
+BackgroundChildImpl::AllocPBackgroundStorageChild(const nsString& aProfilePath)
 {
   MOZ_CRASH("PBackgroundStorageChild actors should be manually constructed!");
 }
 
 bool
 BackgroundChildImpl::DeallocPBackgroundStorageChild(
                                                 PBackgroundStorageChild* aActor)
 {
--- a/ipc/glue/BackgroundChildImpl.h
+++ b/ipc/glue/BackgroundChildImpl.h
@@ -66,17 +66,17 @@ protected:
   virtual PBackgroundIndexedDBUtilsChild*
   AllocPBackgroundIndexedDBUtilsChild() override;
 
   virtual bool
   DeallocPBackgroundIndexedDBUtilsChild(PBackgroundIndexedDBUtilsChild* aActor)
                                         override;
 
   virtual PBackgroundStorageChild*
-  AllocPBackgroundStorageChild() override;
+  AllocPBackgroundStorageChild(const nsString& aProfilePath) override;
 
   virtual bool
   DeallocPBackgroundStorageChild(PBackgroundStorageChild* aActor) override;
 
   virtual PPendingIPCBlobChild*
   AllocPPendingIPCBlobChild(const IPCBlob& aBlob) override;
 
   virtual bool
--- a/ipc/glue/BackgroundParentImpl.cpp
+++ b/ipc/glue/BackgroundParentImpl.cpp
@@ -239,34 +239,35 @@ BackgroundParentImpl::RecvFlushPendingFi
 
   if (!mozilla::dom::indexedDB::RecvFlushPendingFileDeletions()) {
     return IPC_FAIL_NO_REASON(this);
   }
   return IPC_OK();
 }
 
 auto
-BackgroundParentImpl::AllocPBackgroundStorageParent()
+BackgroundParentImpl::AllocPBackgroundStorageParent(const nsString& aProfilePath)
   -> PBackgroundStorageParent*
 {
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
 
-  return mozilla::dom::AllocPBackgroundStorageParent();
+  return mozilla::dom::AllocPBackgroundStorageParent(aProfilePath);
 }
 
 mozilla::ipc::IPCResult
 BackgroundParentImpl::RecvPBackgroundStorageConstructor(
-                                               PBackgroundStorageParent* aActor)
+                                               PBackgroundStorageParent* aActor,
+                                               const nsString& aProfilePath)
 {
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aActor);
 
-  return mozilla::dom::RecvPBackgroundStorageConstructor(aActor);
+  return mozilla::dom::RecvPBackgroundStorageConstructor(aActor, aProfilePath);
 }
 
 bool
 BackgroundParentImpl::DeallocPBackgroundStorageParent(
                                                PBackgroundStorageParent* aActor)
 {
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
--- a/ipc/glue/BackgroundParentImpl.h
+++ b/ipc/glue/BackgroundParentImpl.h
@@ -59,20 +59,21 @@ protected:
   DeallocPBackgroundIndexedDBUtilsParent(
                                         PBackgroundIndexedDBUtilsParent* aActor)
                                         override;
 
   virtual mozilla::ipc::IPCResult
   RecvFlushPendingFileDeletions() override;
 
   virtual PBackgroundStorageParent*
-  AllocPBackgroundStorageParent() override;
+  AllocPBackgroundStorageParent(const nsString& aProfilePath) override;
 
   virtual mozilla::ipc::IPCResult
-  RecvPBackgroundStorageConstructor(PBackgroundStorageParent* aActor) override;
+  RecvPBackgroundStorageConstructor(PBackgroundStorageParent* aActor,
+                                    const nsString& aProfilePath) override;
 
   virtual bool
   DeallocPBackgroundStorageParent(PBackgroundStorageParent* aActor) override;
 
   virtual PPendingIPCBlobParent*
   AllocPPendingIPCBlobParent(const IPCBlob& aBlob) override;
 
   virtual bool
--- a/ipc/glue/PBackground.ipdl
+++ b/ipc/glue/PBackground.ipdl
@@ -83,17 +83,17 @@ parent:
 
   async PBackgroundIDBFactory(LoggingInfo loggingInfo);
 
   async PBackgroundIndexedDBUtils();
 
   // Use only for testing!
   async FlushPendingFileDeletions();
 
-  async PBackgroundStorage();
+  async PBackgroundStorage(nsString profilePath);
 
   async PVsync();
 
   async PCameras();
 
   async PUDPSocket(OptionalPrincipalInfo pInfo, nsCString filter);
   async PBroadcastChannel(PrincipalInfo pInfo, nsCString origin, nsString channel);