Bug 1151242 - Abort version change transactions more eagerly in the event of a crash, r=khuey.
☠☠ backed out by cae04b7cf1dd ☠ ☠
authorBen Turner <bent.mozilla@gmail.com>
Tue, 14 Apr 2015 10:02:54 -0700
changeset 270410 dd10ce7a6b1af027ec415ffed07e694557e0f97d
parent 270409 879e74f2e74cc56c507c7784f05020462c7ad734
child 270411 9edcd70834db4762bf5bb4dc5850725298a47e04
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1151242
milestone40.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 1151242 - Abort version change transactions more eagerly in the event of a crash, r=khuey.
dom/indexedDB/ActorsParent.cpp
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -6586,33 +6586,40 @@ class OpenDatabaseOp final
   nsString mDatabaseFilePath;
   nsRefPtr<FileManager> mFileManager;
 
   nsRefPtr<Database> mDatabase;
   nsRefPtr<VersionChangeTransaction> mVersionChangeTransaction;
 
   nsRefPtr<DatabaseOfflineStorage> mOfflineStorage;
 
+  // This is only set while a VersionChangeOp is live. It holds a strong
+  // reference to its OpenDatabaseOp object so this is a weak pointer to avoid
+  // cycles.
+  VersionChangeOp* mVersionChangeOp;
+
 public:
   OpenDatabaseOp(Factory* aFactory,
                  already_AddRefed<ContentParent> aContentParent,
                  const CommonFactoryRequestParams& aParams);
 
   bool
   IsOtherProcessActor() const
   {
     MOZ_ASSERT(mOptionalContentParentId.type() != OptionalContentId::T__None);
 
     return mOptionalContentParentId.type() ==
              OptionalContentId::TContentParentId;
   }
 
 private:
   ~OpenDatabaseOp()
-  { }
+  {
+    MOZ_ASSERT(!mVersionChangeOp);
+  }
 
   nsresult
   LoadDatabaseInformation(mozIStorageConnection* aConnection);
 
   nsresult
   SendUpgradeNeeded();
 
   void
@@ -6630,16 +6637,19 @@ private:
   ;
 #else
   { }
 #endif
 
   void
   ConnectionClosedCallback();
 
+  virtual void
+  ActorDestroy(ActorDestroyReason aWhy) override;
+
   virtual nsresult
   QuotaManagerOpen() override;
 
   virtual nsresult
   DoDatabaseWork() override;
 
   virtual nsresult
   BeginVersionChange() override;
@@ -6679,17 +6689,19 @@ private:
     , mRequestedVersion(aOpenDatabaseOp->mRequestedVersion)
     , mPreviousVersion(aOpenDatabaseOp->mMetadata->mCommonMetadata.version())
   {
     MOZ_ASSERT(aOpenDatabaseOp);
     MOZ_ASSERT(mRequestedVersion);
   }
 
   ~VersionChangeOp()
-  { }
+  {
+    MOZ_ASSERT(!mOpenDatabaseOp);
+  }
 
   virtual nsresult
   DoDatabaseWork(DatabaseConnection* aConnection) override;
 
   virtual nsresult
   SendSuccessResult() override;
 
   virtual bool
@@ -7903,17 +7915,17 @@ class DatabaseOfflineStorage final
   const nsCString mId;
   nsCOMPtr<nsIEventTarget> mOwningThread;
 
   bool mClosedOnMainThread;
   bool mClosedOnOwningThread;
   bool mInvalidatedOnMainThread;
   bool mInvalidatedOnOwningThread;
 
-  DebugOnly<bool> mRegisteredWithQuotaManager;
+  bool mRegisteredWithQuotaManager;
 
 public:
   DatabaseOfflineStorage(QuotaClient* aQuotaClient,
                          const OptionalContentId& aOptionalContentParentId,
                          const nsACString& aGroup,
                          const nsACString& aOrigin,
                          const nsACString& aId,
                          PersistenceType aPersistenceType,
@@ -7959,17 +7971,17 @@ public:
   }
 
   NS_DECL_THREADSAFE_ISUPPORTS
 
 private:
   ~DatabaseOfflineStorage()
   {
     MOZ_ASSERT(!mDatabase);
-    MOZ_ASSERT(!mRegisteredWithQuotaManager);
+    MOZ_RELEASE_ASSERT(!mRegisteredWithQuotaManager);
   }
 
   void
   CloseOnMainThread();
 
   void
   InvalidateOnMainThread();
 
@@ -8449,20 +8461,21 @@ DatabaseConnection::BeginWriteTransactio
   return NS_OK;
 }
 
 void
 DatabaseConnection::FinishWriteTransaction()
 {
   AssertIsOnConnectionThread();
   MOZ_ASSERT(mStorageConnection);
-  MOZ_ASSERT(mUpdateRefcountFunction);
   MOZ_ASSERT(mDEBUGInWriteTransaction);
 
-  mUpdateRefcountFunction->Reset();
+  if (mUpdateRefcountFunction) {
+    mUpdateRefcountFunction->Reset();
+  }
 
 #ifdef DEBUG
   mDEBUGInWriteTransaction = false;
 #endif
 
   CachedStatement stmt;
   nsresult rv = GetCachedStatement("BEGIN", &stmt);
   if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -17090,29 +17103,42 @@ FactoryOp::RecvPermissionRetry()
 }
 
 OpenDatabaseOp::OpenDatabaseOp(Factory* aFactory,
                                already_AddRefed<ContentParent> aContentParent,
                                const CommonFactoryRequestParams& aParams)
   : FactoryOp(aFactory, Move(aContentParent), aParams, /* aDeleting */ false)
   , mMetadata(new FullDatabaseMetadata(aParams.metadata()))
   , mRequestedVersion(aParams.metadata().version())
+  , mVersionChangeOp(nullptr)
 {
   auto& optionalContentParentId =
     const_cast<OptionalContentId&>(mOptionalContentParentId);
 
   if (mContentParent) {
     // This is a little scary but it looks safe to call this off the main thread
     // for now.
     optionalContentParentId = mContentParent->ChildID();
   } else {
     optionalContentParentId = void_t();
   }
 }
 
+void
+OpenDatabaseOp::ActorDestroy(ActorDestroyReason aWhy)
+{
+  AssertIsOnOwningThread();
+
+  if (mVersionChangeOp) {
+    mVersionChangeOp->NoteActorDestroyed();
+  }
+
+  FactoryOp::ActorDestroy(aWhy);
+}
+
 nsresult
 OpenDatabaseOp::QuotaManagerOpen()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mState == State_OpenPending);
   MOZ_ASSERT(!mOfflineStorage);
 
   QuotaClient* quotaClient = QuotaClient::GetInstance();
@@ -17746,33 +17772,34 @@ OpenDatabaseOp::DispatchToWorkThread()
   // Intentionally empty.
   nsTArray<nsString> objectStoreNames;
 
   const int64_t loggingSerialNumber =
     mVersionChangeTransaction->LoggingSerialNumber();
   const nsID& backgroundChildLoggingId =
     mVersionChangeTransaction->GetLoggingInfo()->Id();
 
+  if (NS_WARN_IF(!mDatabase->RegisterTransaction(mVersionChangeTransaction))) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
   nsRefPtr<VersionChangeOp> versionChangeOp = new VersionChangeOp(this);
 
   uint64_t transactionId =
     gConnectionPool->Start(backgroundChildLoggingId,
                            mVersionChangeTransaction->DatabaseId(),
                            loggingSerialNumber,
                            objectStoreNames,
                            /* aIsWriteTransaction */ true,
                            versionChangeOp);
 
-  mVersionChangeTransaction->SetActive(transactionId);
+  mVersionChangeOp = versionChangeOp;
 
   mVersionChangeTransaction->NoteActiveRequest();
-
-  if (NS_WARN_IF(!mDatabase->RegisterTransaction(mVersionChangeTransaction))) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+  mVersionChangeTransaction->SetActive(transactionId);
 
   return NS_OK;
 }
 
 nsresult
 OpenDatabaseOp::SendUpgradeNeeded()
 {
   AssertIsOnOwningThread();
@@ -18217,16 +18244,17 @@ OpenDatabaseOp::AssertMetadataConsistenc
 #endif // DEBUG
 
 nsresult
 OpenDatabaseOp::
 VersionChangeOp::DoDatabaseWork(DatabaseConnection* aConnection)
 {
   MOZ_ASSERT(aConnection);
   aConnection->AssertIsOnConnectionThread();
+  MOZ_ASSERT(mOpenDatabaseOp);
   MOZ_ASSERT(mOpenDatabaseOp->mState == State_DatabaseWorkVersionChange);
 
   if (NS_WARN_IF(QuotaClient::IsShuttingDownOnNonMainThread()) ||
       !OperationMayProceed()) {
     IDB_REPORT_INTERNAL_ERR();
     return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
   }
 
@@ -18271,47 +18299,52 @@ VersionChangeOp::DoDatabaseWork(Database
 
 nsresult
 OpenDatabaseOp::
 VersionChangeOp::SendSuccessResult()
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mOpenDatabaseOp);
   MOZ_ASSERT(mOpenDatabaseOp->mState == State_DatabaseWorkVersionChange);
+  MOZ_ASSERT(mOpenDatabaseOp->mVersionChangeOp == this);
 
   nsresult rv = mOpenDatabaseOp->SendUpgradeNeeded();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   return NS_OK;
 }
 
 bool
 OpenDatabaseOp::
 VersionChangeOp::SendFailureResult(nsresult aResultCode)
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mOpenDatabaseOp);
   MOZ_ASSERT(mOpenDatabaseOp->mState == State_DatabaseWorkVersionChange);
+  MOZ_ASSERT(mOpenDatabaseOp->mVersionChangeOp == this);
 
   mOpenDatabaseOp->SetFailureCode(aResultCode);
   mOpenDatabaseOp->mState = State_SendingResults;
 
   MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mOpenDatabaseOp->Run()));
 
   return false;
 }
 
 void
 OpenDatabaseOp::
 VersionChangeOp::Cleanup()
 {
   AssertIsOnOwningThread();
-
+  MOZ_ASSERT(mOpenDatabaseOp);
+  MOZ_ASSERT(mOpenDatabaseOp->mVersionChangeOp == this);
+
+  mOpenDatabaseOp->mVersionChangeOp = nullptr;
   mOpenDatabaseOp = nullptr;
 
 #ifdef DEBUG
   // A bit hacky but the VersionChangeOp is not generated in response to a
   // child request like most other database operations. Do this to make our
   // assertions happy.
   NoteActorDestroyed();
 #endif
@@ -19338,23 +19371,25 @@ CommitOp::Run()
                mTransaction->LoggingSerialNumber(),
                mLoggingSerialNumber);
 
   if (mTransaction->GetMode() != IDBTransaction::READ_ONLY) {
     Database* database = mTransaction->GetDatabase();
     MOZ_ASSERT(database);
 
     if (DatabaseConnection* connection = database->GetConnection()) {
+      // May be null if the VersionChangeOp was canceled.
       DatabaseConnection::UpdateRefcountFunction* fileRefcountFunction =
         connection->GetUpdateRefcountFunction();
-      MOZ_ASSERT(fileRefcountFunction);
 
       if (NS_SUCCEEDED(mResultCode)) {
-        mResultCode = fileRefcountFunction->WillCommit();
-        NS_WARN_IF_FALSE(NS_SUCCEEDED(mResultCode), "WillCommit() failed!");
+        if (fileRefcountFunction) {
+          mResultCode = fileRefcountFunction->WillCommit();
+          NS_WARN_IF_FALSE(NS_SUCCEEDED(mResultCode), "WillCommit() failed!");
+        }
 
         if (NS_SUCCEEDED(mResultCode)) {
           mResultCode = WriteAutoIncrementCounts();
           NS_WARN_IF_FALSE(NS_SUCCEEDED(mResultCode),
                            "WriteAutoIncrementCounts() failed!");
 
           if (NS_SUCCEEDED(mResultCode)) {
             AssertForeignKeyConsistency(connection);
@@ -19368,26 +19403,28 @@ CommitOp::Run()
               mResultCode = stmt->Execute();
               NS_WARN_IF_FALSE(NS_SUCCEEDED(mResultCode), "Commit failed!");
 
               if (mTransaction->GetMode() == IDBTransaction::READ_WRITE_FLUSH &&
                   NS_SUCCEEDED(mResultCode)) {
                 mResultCode = connection->Checkpoint(/* aIdle */ false);
               }
 
-              if (NS_SUCCEEDED(mResultCode)) {
+              if (NS_SUCCEEDED(mResultCode) && fileRefcountFunction) {
                 fileRefcountFunction->DidCommit();
               }
             }
           }
         }
       }
 
       if (NS_FAILED(mResultCode)) {
-        fileRefcountFunction->DidAbort();
+        if (fileRefcountFunction) {
+          fileRefcountFunction->DidAbort();
+        }
 
         DatabaseConnection::CachedStatement stmt;
         if (NS_SUCCEEDED(connection->GetCachedStatement("ROLLBACK", &stmt))) {
           // This may fail if SQLite already rolled back the transaction so
           // ignore any errors.
           unused << stmt->Execute();
         } else {
           NS_WARNING("Failed to prepare ROLLBACK statement!");