Bug 1411908 - Part 2: Wait for all factory operations to finish when shutting down; r=asuth
authorJan Varga <jan.varga@gmail.com>
Fri, 04 May 2018 06:32:23 +0200
changeset 416896 f579a28444855e4ccbf45c8c82f9bb78ebbe822b
parent 416895 1a0c61c2cf60a2c0e4bf7035effd467dd6185f65
child 416897 8994f35fe5fc89f4e8f4e09579a6962f8a4a3e65
child 416930 00725c5bba29576e85aab17f4d18a67ef2f5c327
push id33941
push usershindli@mozilla.com
push dateFri, 04 May 2018 08:36:02 +0000
treeherdermozilla-central@8994f35fe5fc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1411908
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 1411908 - Part 2: Wait for all factory operations to finish when shutting down; r=asuth
dom/indexedDB/ActorsParent.cpp
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -7464,17 +7464,16 @@ protected:
   nsCString mSuffix;
   nsCString mGroup;
   nsCString mOrigin;
   nsCString mDatabaseId;
   nsString mDatabaseFilePath;
   State mState;
   bool mEnforcingQuota;
   const bool mDeleting;
-  bool mBlockedDatabaseOpen;
   bool mChromeWriteAccessAllowed;
   bool mFileHandleDisabled;
 
 public:
   void
   NoteDatabaseBlocked(Database* aDatabase);
 
   virtual void
@@ -7483,19 +7482,30 @@ public:
 #ifdef DEBUG
   bool
   HasBlockedDatabases() const
   {
     return !mMaybeBlockedDatabases.IsEmpty();
   }
 #endif
 
+  bool
+  DatabaseFilePathIsKnown() const
+  {
+    AssertIsOnOwningThread();
+
+    return !mDatabaseFilePath.IsEmpty();
+  }
+
   const nsString&
   DatabaseFilePath() const
   {
+    AssertIsOnOwningThread();
+    MOZ_ASSERT(!mDatabaseFilePath.IsEmpty());
+
     return mDatabaseFilePath;
   }
 
 protected:
   FactoryOp(Factory* aFactory,
             already_AddRefed<ContentParent> aContentParent,
             const CommonFactoryRequestParams& aCommonParams,
             bool aDeleting);
@@ -7522,16 +7532,19 @@ protected:
 
   nsresult
   SendToIOThread();
 
   void
   WaitForTransactions();
 
   void
+  CleanupMetadata();
+
+  void
   FinishSendResults();
 
   nsresult
   SendVersionChangeMessages(DatabaseActorInfo* aDatabaseActorInfo,
                             Database* aOpeningDatabase,
                             uint64_t aOldVersion,
                             const NullableVersion& aNewVersion);
 
@@ -13681,16 +13694,21 @@ Factory::AllocPBackgroundIDBFactoryReque
   if (aParams.type() == FactoryRequestParams::TOpenDatabaseRequestParams) {
     actor = new OpenDatabaseOp(this,
                                contentParent.forget(),
                                *commonParams);
   } else {
     actor = new DeleteDatabaseOp(this, contentParent.forget(), *commonParams);
   }
 
+  gFactoryOps->AppendElement(actor);
+
+  // Balanced in CleanupMetadata() which is/must always called by SendResults().
+  IncreaseBusyCount();
+
   // Transfer ownership to IPDL.
   return actor.forget().take();
 }
 
 mozilla::ipc::IPCResult
 Factory::RecvPBackgroundIDBFactoryRequestConstructor(
                                      PBackgroundIDBFactoryRequestParent* aActor,
                                      const FactoryRequestParams& aParams)
@@ -17879,17 +17897,18 @@ QuotaClient::ShutdownWorkThreads()
   MOZ_ASSERT(!mShutdownRequested);
 
   mShutdownRequested = true;
 
   AbortOperations(VoidCString());
 
   // This should release any IDB related quota objects or directory locks.
   MOZ_ALWAYS_TRUE(SpinEventLoopUntil([&]() {
-    return (!gLiveDatabaseHashtable || !gLiveDatabaseHashtable->Count()) &&
+    return (!gFactoryOps || gFactoryOps->IsEmpty()) &&
+           (!gLiveDatabaseHashtable || !gLiveDatabaseHashtable->Count()) &&
            !mCurrentMaintenance;
   }));
 
   // And finally, shutdown all threads.
   RefPtr<ConnectionPool> connectionPool = gConnectionPool.get();
   if (connectionPool) {
     connectionPool->Shutdown();
 
@@ -18636,17 +18655,19 @@ Maintenance::BeginDatabaseMaintenance()
   public:
     static bool
     IsSafeToRunMaintenance(const nsAString& aDatabasePath)
     {
       if (gFactoryOps) {
         for (uint32_t index = gFactoryOps->Length(); index > 0; index--) {
           RefPtr<FactoryOp>& existingOp = (*gFactoryOps)[index - 1];
 
-          MOZ_ASSERT(!existingOp->DatabaseFilePath().IsEmpty());
+          if (!existingOp->DatabaseFilePathIsKnown()) {
+            continue;
+          }
 
           if (existingOp->DatabaseFilePath() == aDatabasePath) {
             return false;
           }
         }
       }
 
       if (gLiveDatabaseHashtable) {
@@ -20683,17 +20704,16 @@ FactoryOp::FactoryOp(Factory* aFactory,
   : DatabaseOperationBase(aFactory->GetLoggingInfo()->Id(),
                           aFactory->GetLoggingInfo()->NextRequestSN())
   , mFactory(aFactory)
   , mContentParent(Move(aContentParent))
   , mCommonParams(aCommonParams)
   , mState(State::Initial)
   , mEnforcingQuota(true)
   , mDeleting(aDeleting)
-  , mBlockedDatabaseOpen(false)
   , mChromeWriteAccessAllowed(false)
   , mFileHandleDisabled(false)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aFactory);
   MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread());
 }
 
@@ -20826,59 +20846,52 @@ FactoryOp::RetryCheckPermission()
 
 nsresult
 FactoryOp::DirectoryOpen()
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mState == State::DirectoryOpenPending);
   MOZ_ASSERT(mDirectoryLock);
   MOZ_ASSERT(!mDatabaseFilePath.IsEmpty());
-
-  // gFactoryOps could be null here if the child process crashed or something
-  // and that cleaned up the last Factory actor.
-  if (!gFactoryOps) {
-    return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
-  }
+  MOZ_ASSERT(gFactoryOps);
 
   // See if this FactoryOp needs to wait.
   bool delayed = false;
+  bool foundThis = false;
   for (uint32_t index = gFactoryOps->Length(); index > 0; index--) {
     RefPtr<FactoryOp>& existingOp = (*gFactoryOps)[index - 1];
-    if (MustWaitFor(*existingOp)) {
+
+    if (existingOp == this) {
+      foundThis = true;
+      continue;
+    }
+
+    if (foundThis && MustWaitFor(*existingOp)) {
       // Only one op can be delayed.
       MOZ_ASSERT(!existingOp->mDelayedOp);
       existingOp->mDelayedOp = this;
       delayed = true;
       break;
     }
   }
 
-  // Adding this to the factory ops list will block any additional ops from
-  // proceeding until this one is done.
-  gFactoryOps->AppendElement(this);
-
   if (!delayed) {
     QuotaClient* quotaClient = QuotaClient::GetInstance();
     MOZ_ASSERT(quotaClient);
 
     if (RefPtr<Maintenance> currentMaintenance =
           quotaClient->GetCurrentMaintenance()) {
       if (RefPtr<DatabaseMaintenance> databaseMaintenance =
             currentMaintenance->GetDatabaseMaintenance(mDatabaseFilePath)) {
         databaseMaintenance->WaitForCompletion(this);
         delayed = true;
       }
     }
   }
 
-  mBlockedDatabaseOpen = true;
-
-  // Balanced in FinishSendResults().
-  IncreaseBusyCount();
-
   mState = State::DatabaseOpenPending;
   if (!delayed) {
     nsresult rv = DatabaseOpen();
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
@@ -20924,38 +20937,42 @@ FactoryOp::WaitForTransactions()
   mState = State::WaitingForTransactionsToComplete;
 
   RefPtr<WaitForTransactionsHelper> helper =
     new WaitForTransactionsHelper(mDatabaseId, this);
   helper->WaitForTransactions();
 }
 
 void
+FactoryOp::CleanupMetadata()
+{
+  AssertIsOnOwningThread();
+
+  if (mDelayedOp) {
+    MOZ_ALWAYS_SUCCEEDS(NS_DispatchToCurrentThread(mDelayedOp.forget()));
+  }
+
+  MOZ_ASSERT(gFactoryOps);
+  gFactoryOps->RemoveElement(this);
+
+  // Match the IncreaseBusyCount in AllocPBackgroundIDBFactoryRequestParent().
+  DecreaseBusyCount();
+}
+
+void
 FactoryOp::FinishSendResults()
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mState == State::SendingResults);
   MOZ_ASSERT(mFactory);
 
   // Make sure to release the factory on this thread.
   RefPtr<Factory> factory;
   mFactory.swap(factory);
 
-  if (mBlockedDatabaseOpen) {
-    if (mDelayedOp) {
-      MOZ_ALWAYS_SUCCEEDS(NS_DispatchToCurrentThread(mDelayedOp.forget()));
-    }
-
-    MOZ_ASSERT(gFactoryOps);
-    gFactoryOps->RemoveElement(this);
-
-    // Match the IncreaseBusyCount in DirectoryOpen().
-    DecreaseBusyCount();
-  }
-
   mState = State::Completed;
 }
 
 nsresult
 FactoryOp::CheckPermission(ContentParent* aContentParent,
                            PermissionRequestBase::PermissionValue* aPermission)
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -22427,38 +22444,45 @@ OpenDatabaseOp::SendResults()
     MOZ_ASSERT(!mDirectoryLock);
 
     if (NS_FAILED(mResultCode)) {
       mDatabase->Invalidate();
     }
 
     // Make sure to release the database on this thread.
     mDatabase = nullptr;
+
+    CleanupMetadata();
   } else if (mDirectoryLock) {
+    // ConnectionClosedCallback will call CleanupMetadata().
     nsCOMPtr<nsIRunnable> callback = NewRunnableMethod(
       "dom::indexedDB::OpenDatabaseOp::ConnectionClosedCallback",
       this,
       &OpenDatabaseOp::ConnectionClosedCallback);
 
     RefPtr<WaitForTransactionsHelper> helper =
       new WaitForTransactionsHelper(mDatabaseId, callback);
     helper->WaitForTransactions();
+  } else {
+    CleanupMetadata();
   }
 
   FinishSendResults();
 }
 
 void
 OpenDatabaseOp::ConnectionClosedCallback()
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(NS_FAILED(mResultCode));
   MOZ_ASSERT(mDirectoryLock);
 
   mDirectoryLock = nullptr;
+
+  CleanupMetadata();
 }
 
 void
 OpenDatabaseOp::EnsureDatabaseActor()
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mState == State::BeginVersionChange ||
              mState == State::DatabaseWorkVersionChange ||
@@ -23102,16 +23126,18 @@ DeleteDatabaseOp::SendResults()
     }
 
     Unused <<
       PBackgroundIDBFactoryRequestParent::Send__delete__(this, response);
   }
 
   mDirectoryLock = nullptr;
 
+  CleanupMetadata();
+
   FinishSendResults();
 }
 
 nsresult
 DeleteDatabaseOp::
 VersionChangeOp::DeleteFile(nsIFile* aDirectory,
                             const nsAString& aFilename,
                             QuotaManager* aQuotaManager)