Bug 965084 - "ROLLBACK TO SAVEPOINT savepoint" doesn't fire appropriate triggers leaving orphaned files. r=bent
authorJan Varga <jan.varga@gmail.com>
Fri, 31 Jan 2014 10:53:37 -0800
changeset 166296 35c57af24bf5e00d401ed2699379ad606e6ef6df
parent 166295 78bbd153072a539d534a2c2c71e34a2afb49eb9c
child 166297 7400843fb1c0b718c60172c77644f61f0a49dfb6
push id39166
push userJan.Varga@gmail.com
push dateFri, 31 Jan 2014 18:54:01 +0000
treeherdermozilla-inbound@35c57af24bf5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs965084
milestone29.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 965084 - "ROLLBACK TO SAVEPOINT savepoint" doesn't fire appropriate triggers leaving orphaned files. r=bent
dom/indexedDB/IDBTransaction.cpp
dom/indexedDB/IDBTransaction.h
dom/indexedDB/test/test_file_os_delete.html
--- a/dom/indexedDB/IDBTransaction.cpp
+++ b/dom/indexedDB/IDBTransaction.cpp
@@ -293,16 +293,20 @@ IDBTransaction::StartSavepoint()
   ));
   NS_ENSURE_TRUE(stmt, false);
 
   mozStorageStatementScoper scoper(stmt);
 
   nsresult rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, false);
 
+  if (IsWriteAllowed()) {
+    mUpdateFileRefcountFunction->StartSavepoint();
+  }
+
   ++mSavepointCount;
 
   return true;
 }
 
 nsresult
 IDBTransaction::ReleaseSavepoint()
 {
@@ -316,16 +320,20 @@ IDBTransaction::ReleaseSavepoint()
   ));
   NS_ENSURE_TRUE(stmt, NS_OK);
 
   mozStorageStatementScoper scoper(stmt);
 
   nsresult rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, NS_OK);
 
+  if (IsWriteAllowed()) {
+    mUpdateFileRefcountFunction->ReleaseSavepoint();
+  }
+
   --mSavepointCount;
 
   return NS_OK;
 }
 
 void
 IDBTransaction::RollbackSavepoint()
 {
@@ -339,16 +347,20 @@ IDBTransaction::RollbackSavepoint()
     "ROLLBACK TO SAVEPOINT " SAVEPOINT_NAME
   ));
   NS_ENSURE_TRUE_VOID(stmt);
 
   mozStorageStatementScoper scoper(stmt);
 
   nsresult rv = stmt->Execute();
   NS_ENSURE_SUCCESS_VOID(rv);
+
+  if (IsWriteAllowed()) {
+    mUpdateFileRefcountFunction->RollbackSavepoint();
+  }
 }
 
 nsresult
 IDBTransaction::GetOrCreateConnection(mozIStorageConnection** aResult)
 {
   NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
   NS_ASSERTION(IndexedDatabaseManager::IsMainProcess(), "Wrong process!");
 
@@ -1073,22 +1085,32 @@ UpdateRefcountFunction::ProcessValue(moz
       nsRefPtr<FileInfo> fileInfo = mFileManager->GetFileInfo(id);
       NS_ASSERTION(fileInfo, "Shouldn't be null!");
 
       nsAutoPtr<FileInfoEntry> newEntry(new FileInfoEntry(fileInfo));
       mFileInfoEntries.Put(id, newEntry);
       entry = newEntry.forget();
     }
 
+    if (mInSavepoint) {
+      mSavepointEntriesIndex.Put(id, entry);
+    }
+
     switch (aUpdateType) {
       case eIncrement:
         entry->mDelta++;
+        if (mInSavepoint) {
+          entry->mSavepointDelta++;
+        }
         break;
       case eDecrement:
         entry->mDelta--;
+        if (mInSavepoint) {
+          entry->mSavepointDelta--;
+        }
         break;
       default:
         NS_NOTREACHED("Unknown update type!");
     }
   }
 
   return NS_OK;
 }
@@ -1160,16 +1182,26 @@ UpdateRefcountFunction::FileInfoUpdateCa
 {
   if (aValue->mDelta) {
     aValue->mFileInfo->UpdateDBRefs(aValue->mDelta);
   }
 
   return PL_DHASH_NEXT;
 }
 
+PLDHashOperator
+UpdateRefcountFunction::RollbackSavepointCallback(const uint64_t& aKey,
+                                                  FileInfoEntry* aValue,
+                                                  void* aUserArg)
+{
+  aValue->mDelta -= aValue->mSavepointDelta;
+
+  return PL_DHASH_NEXT;
+}
+
 bool
 UpdateRefcountFunction::DatabaseUpdateFunction::Update(int64_t aId,
                                                        int32_t aDelta)
 {
   nsresult rv = UpdateInternal(aId, aDelta);
   if (NS_FAILED(rv)) {
     mErrorCode = rv;
     return false;
--- a/dom/indexedDB/IDBTransaction.h
+++ b/dom/indexedDB/IDBTransaction.h
@@ -362,44 +362,73 @@ private:
 
 class UpdateRefcountFunction MOZ_FINAL : public mozIStorageFunction
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_MOZISTORAGEFUNCTION
 
   UpdateRefcountFunction(FileManager* aFileManager)
-  : mFileManager(aFileManager)
+  : mFileManager(aFileManager), mInSavepoint(false)
   { }
 
   ~UpdateRefcountFunction()
   { }
 
+  void StartSavepoint()
+  {
+    MOZ_ASSERT(!mInSavepoint);
+    MOZ_ASSERT(!mSavepointEntriesIndex.Count());
+
+    mInSavepoint = true;
+  }
+
+  void ReleaseSavepoint()
+  {
+    MOZ_ASSERT(mInSavepoint);
+
+    mSavepointEntriesIndex.Clear();
+
+    mInSavepoint = false;
+  }
+
+  void RollbackSavepoint()
+  {
+    MOZ_ASSERT(mInSavepoint);
+
+    mInSavepoint = false;
+
+    mSavepointEntriesIndex.EnumerateRead(RollbackSavepointCallback, nullptr);
+
+    mSavepointEntriesIndex.Clear();
+  }
+
   void ClearFileInfoEntries()
   {
     mFileInfoEntries.Clear();
   }
 
   nsresult WillCommit(mozIStorageConnection* aConnection);
   void DidCommit();
   void DidAbort();
 
 private:
   class FileInfoEntry
   {
   public:
     FileInfoEntry(FileInfo* aFileInfo)
-    : mFileInfo(aFileInfo), mDelta(0)
+    : mFileInfo(aFileInfo), mDelta(0), mSavepointDelta(0)
     { }
 
     ~FileInfoEntry()
     { }
 
     nsRefPtr<FileInfo> mFileInfo;
     int32_t mDelta;
+    int32_t mSavepointDelta;
   };
 
   enum UpdateType {
     eIncrement,
     eDecrement
   };
 
   class DatabaseUpdateFunction
@@ -442,19 +471,27 @@ private:
                          FileInfoEntry* aValue,
                          void* aUserArg);
 
   static PLDHashOperator
   FileInfoUpdateCallback(const uint64_t& aKey,
                          FileInfoEntry* aValue,
                          void* aUserArg);
 
+  static PLDHashOperator
+  RollbackSavepointCallback(const uint64_t& aKey,
+                            FileInfoEntry* aValue,
+                            void* aUserArg);
+
   FileManager* mFileManager;
   nsClassHashtable<nsUint64HashKey, FileInfoEntry> mFileInfoEntries;
+  nsDataHashtable<nsUint64HashKey, FileInfoEntry*> mSavepointEntriesIndex;
 
   nsTArray<int64_t> mJournalsToCreateBeforeCommit;
   nsTArray<int64_t> mJournalsToRemoveAfterCommit;
   nsTArray<int64_t> mJournalsToRemoveAfterAbort;
+
+  bool mInSavepoint;
 };
 
 END_INDEXEDDB_NAMESPACE
 
 #endif // mozilla_dom_indexeddb_idbtransaction_h__
--- a/dom/indexedDB/test/test_file_os_delete.html
+++ b/dom/indexedDB/test/test_file_os_delete.html
@@ -16,65 +16,82 @@
 
     const name = window.location.pathname;
 
     const objectStoreName = "Blobs";
 
     getUsage(grabFileUsageAndContinueHandler);
     let startUsage = yield undefined;
 
-    const fileData = { key: 1, file: getRandomFile("random.bin", 100000) };
+    const fileData1 = {
+      key: 1,
+      obj: { id: 1, file: getRandomFile("random.bin", 100000) }
+    };
+    const fileData2 = {
+      key: 2,
+      obj: { id: 1, file: getRandomFile("random.bin", 100000) }
+    };
 
     {
       let request = indexedDB.open(name, 1);
       request.onerror = errorHandler;
       request.onupgradeneeded = grabEventAndContinueHandler;
       request.onsuccess = grabEventAndContinueHandler;
       let event = yield undefined;
 
       is(event.type, "upgradeneeded", "Got correct event type");
 
       let db = event.target.result;
       db.onerror = errorHandler;
 
       let objectStore = db.createObjectStore(objectStoreName, { });
 
-      objectStore.add(fileData.file, fileData.key);
+      objectStore.createIndex("index", "id", { unique: true });
+
+      objectStore.add(fileData1.obj, fileData1.key);
+
+      request = objectStore.add(fileData2.obj, fileData2.key);
+      request.addEventListener("error", new ExpectError("ConstraintError", true));
+      request.onsuccess = unexpectedSuccessHandler;
+      yield undefined;
 
       event = yield undefined;
 
       is(event.type, "success", "Got correct event type");
 
       getUsage(grabFileUsageAndContinueHandler);
       let usage = yield undefined;
 
-      is(usage, startUsage + fileData.file.size, "Correct file usage");
+      is(usage, startUsage + fileData1.obj.file.size + fileData2.obj.file.size,
+         "Correct file usage");
 
       let trans = db.transaction([objectStoreName], READ_WRITE);
-      trans.objectStore(objectStoreName).delete(fileData.key);
+      trans.objectStore(objectStoreName).delete(fileData1.key);
       trans.oncomplete = grabEventAndContinueHandler;
       event = yield undefined;
 
       is(event.type, "complete", "Got correct event type");
 
       getUsage(grabFileUsageAndContinueHandler);
       usage = yield undefined;
 
-      is(usage, startUsage + fileData.file.size, "OS file exists");
+      is(usage, startUsage + fileData1.obj.file.size + fileData2.obj.file.size,
+         "OS files exists");
 
-      fileData.file = null;
+      fileData1.obj.file = null;
+      fileData2.obj.file = null;
     }
 
     scheduleGC();
     yield undefined;
 
     getUsage(grabFileUsageAndContinueHandler);
     let endUsage = yield undefined;
 
-    is(endUsage, startUsage, "OS file deleted");
+    is(endUsage, startUsage, "OS files deleted");
 
     finishTest();
     yield undefined;
   }
   </script>
   <script type="text/javascript;version=1.7" src="file.js"></script>
   <script type="text/javascript;version=1.7" src="helpers.js"></script>