IndexedDB: Prevent store/index deletion from racing ahead of use

content/browser/indexed_db/indexed_db_database.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/indexed_db/indexed_db_database.h"
 
 #include <math.h>
 #include <set>
 
 #include "base/auto_reset.h"
@@ skipping 259 matching lines @@
   IndexedDBTransaction* transaction = GetTransaction(transaction_id);
   if (!transaction)
     return;
   DCHECK_EQ(transaction->mode(), indexed_db::TRANSACTION_VERSION_CHANGE);
 
   if (ContainsKey(metadata_.object_stores, object_store_id)) {
     DLOG(ERROR) << "Invalid object_store_id";
     return;
   }
 
+  // Store creation is done synchronously, as it may be followed by
+  // index creation (also sync) since preemptive OpenCursor/SetIndexKeys
+  // may follow.
   IndexedDBObjectStoreMetadata object_store_metadata(
       name,
       object_store_id,
       key_path,
       auto_increment,
       IndexedDBDatabase::kMinimumIndexId);
 
-  transaction->ScheduleTask(
-      base::Bind(&IndexedDBDatabase::CreateObjectStoreOperation,
-                 this,
-                 object_store_metadata),
-      base::Bind(&IndexedDBDatabase::CreateObjectStoreAbortOperation,
-                 this,
-                 object_store_id));
-
-  AddObjectStore(object_store_metadata, object_store_id);
-}
-
-void IndexedDBDatabase::CreateObjectStoreOperation(
-    const IndexedDBObjectStoreMetadata& object_store_metadata,
-    IndexedDBTransaction* transaction) {
-  IDB_TRACE("IndexedDBDatabase::CreateObjectStoreOperation");
   leveldb::Status s =
       backing_store_->CreateObjectStore(transaction->BackingStoreTransaction(),
                                         transaction->database()->id(),
                                         object_store_metadata.id,
                                         object_store_metadata.name,
                                         object_store_metadata.key_path,
                                         object_store_metadata.auto_increment);
   if (!s.ok()) {
     IndexedDBDatabaseError error(
         blink::WebIDBDatabaseExceptionUnknownError,
         ASCIIToUTF16("Internal error creating object store '") +
             object_store_metadata.name + ASCIIToUTF16("'."));
     transaction->Abort(error);
     if (s.IsCorruption())
       factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
                                              error);
     return;
   }
+
+  AddObjectStore(object_store_metadata, object_store_id);
+  transaction->ScheduleAbortTask(
+      base::Bind(&IndexedDBDatabase::CreateObjectStoreAbortOperation,
+                 this,
+                 object_store_id));
 }
 
 void IndexedDBDatabase::DeleteObjectStore(int64 transaction_id,
                                           int64 object_store_id) {
   IDB_TRACE("IndexedDBDatabase::DeleteObjectStore");
   IndexedDBTransaction* transaction = GetTransaction(transaction_id);
   if (!transaction)
     return;
   DCHECK_EQ(transaction->mode(), indexed_db::TRANSACTION_VERSION_CHANGE);
 
   if (!ValidateObjectStoreId(object_store_id))
     return;
 
-  const IndexedDBObjectStoreMetadata& object_store_metadata =
-      metadata_.object_stores[object_store_id];
-
   transaction->ScheduleTask(
       base::Bind(&IndexedDBDatabase::DeleteObjectStoreOperation,
                  this,
-                 object_store_metadata),
-      base::Bind(&IndexedDBDatabase::DeleteObjectStoreAbortOperation,
-                 this,
-                 object_store_metadata));
-  RemoveObjectStore(object_store_id);
+                 object_store_id));
 }
 
 void IndexedDBDatabase::CreateIndex(int64 transaction_id,
                                     int64 object_store_id,
                                     int64 index_id,
                                     const base::string16& name,
                                     const IndexedDBKeyPath& key_path,
                                     bool unique,
                                     bool multi_entry) {
   IDB_TRACE("IndexedDBDatabase::CreateIndex");
   IndexedDBTransaction* transaction = GetTransaction(transaction_id);
   if (!transaction)
     return;
   DCHECK_EQ(transaction->mode(), indexed_db::TRANSACTION_VERSION_CHANGE);
 
   if (!ValidateObjectStoreIdAndNewIndexId(object_store_id, index_id))
     return;
+
+  // Index creation is done synchronously since preemptive
+  // OpenCursor/SetIndexKeys may follow.
   const IndexedDBIndexMetadata index_metadata(
       name, index_id, key_path, unique, multi_entry);
 
-  transaction->ScheduleTask(
-      base::Bind(&IndexedDBDatabase::CreateIndexOperation,
-                 this,
-                 object_store_id,
-                 index_metadata),
-      base::Bind(&IndexedDBDatabase::CreateIndexAbortOperation,
-                 this,
-                 object_store_id,
-                 index_id));
-
-  AddIndex(object_store_id, index_metadata, index_id);
-}
-
-void IndexedDBDatabase::CreateIndexOperation(
-    int64 object_store_id,
-    const IndexedDBIndexMetadata& index_metadata,
-    IndexedDBTransaction* transaction) {
-  IDB_TRACE("IndexedDBDatabase::CreateIndexOperation");
   if (!backing_store_->CreateIndex(transaction->BackingStoreTransaction(),
                                    transaction->database()->id(),
                                    object_store_id,
                                    index_metadata.id,
                                    index_metadata.name,
                                    index_metadata.key_path,
                                    index_metadata.unique,
                                    index_metadata.multi_entry).ok()) {
     base::string16 error_string =
         ASCIIToUTF16("Internal error creating index '") +
         index_metadata.name + ASCIIToUTF16("'.");
     transaction->Abort(IndexedDBDatabaseError(
         blink::WebIDBDatabaseExceptionUnknownError, error_string));
     return;
   }
+
+  AddIndex(object_store_id, index_metadata, index_id);
+  transaction->ScheduleAbortTask(
+      base::Bind(&IndexedDBDatabase::CreateIndexAbortOperation,
+                 this,
+                 object_store_id,
+                 index_id));
 }
 
 void IndexedDBDatabase::CreateIndexAbortOperation(
     int64 object_store_id,
     int64 index_id,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::CreateIndexAbortOperation");
   DCHECK(!transaction);
   RemoveIndex(object_store_id, index_id);
 }
 
 void IndexedDBDatabase::DeleteIndex(int64 transaction_id,
                                     int64 object_store_id,
                                     int64 index_id) {
   IDB_TRACE("IndexedDBDatabase::DeleteIndex");
   IndexedDBTransaction* transaction = GetTransaction(transaction_id);
   if (!transaction)
     return;
   DCHECK_EQ(transaction->mode(), indexed_db::TRANSACTION_VERSION_CHANGE);
 
   if (!ValidateObjectStoreIdAndIndexId(object_store_id, index_id))
     return;
-  const IndexedDBIndexMetadata& index_metadata =
-      metadata_.object_stores[object_store_id].indexes[index_id];
 
   transaction->ScheduleTask(
       base::Bind(&IndexedDBDatabase::DeleteIndexOperation,
                  this,
                  object_store_id,
-                 index_metadata),
-      base::Bind(&IndexedDBDatabase::DeleteIndexAbortOperation,
-                 this,
-                 object_store_id,
-                 index_metadata));
-
-  RemoveIndex(object_store_id, index_id);
+                 index_id));
 }
 
 void IndexedDBDatabase::DeleteIndexOperation(
     int64 object_store_id,
-    const IndexedDBIndexMetadata& index_metadata,
+    int64 index_id,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::DeleteIndexOperation");
+
+  const IndexedDBIndexMetadata index_metadata =
+      metadata_.object_stores[object_store_id].indexes[index_id];
+
   leveldb::Status s =
       backing_store_->DeleteIndex(transaction->BackingStoreTransaction(),
                                   transaction->database()->id(),
                                   object_store_id,
-                                  index_metadata.id);
+                                  index_id);
   if (!s.ok()) {
     base::string16 error_string =
         ASCIIToUTF16("Internal error deleting index '") +
         index_metadata.name + ASCIIToUTF16("'.");
     IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
                                  error_string);
     transaction->Abort(error);
     if (s.IsCorruption())
       factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
                                              error);
+    return;

[jsbell, 2014/05/19 22:07:08]

Seems safer to early exit here; if the backing store can't be updated, leave the
in-memory metadata in place. The abort will restore the metadata on the
front-end as well.

   }
+
+  RemoveIndex(object_store_id, index_id);
+  transaction->ScheduleAbortTask(
+      base::Bind(&IndexedDBDatabase::DeleteIndexAbortOperation,
+                 this,
+                 object_store_id,
+                 index_metadata));
 }
 
 void IndexedDBDatabase::DeleteIndexAbortOperation(
     int64 object_store_id,
     const IndexedDBIndexMetadata& index_metadata,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::DeleteIndexAbortOperation");
   DCHECK(!transaction);
   AddIndex(object_store_id, index_metadata, IndexedDBIndexMetadata::kInvalidId);
 }
@@ skipping 826 matching lines @@
     if (s.IsCorruption()) {
       factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
                                              error);
     }
     return;
   }
   callbacks->OnSuccess();
 }
 
 void IndexedDBDatabase::DeleteObjectStoreOperation(
-    const IndexedDBObjectStoreMetadata& object_store_metadata,
+    int64 object_store_id,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::DeleteObjectStoreOperation");
+
+  const IndexedDBObjectStoreMetadata object_store_metadata =

[cmumford, 2014/05/20 15:29:33]

Can this be a const reference?

[jsbell, 2014/05/20 15:34:43]

No, because metadata_ is mutated by RemoveObjectStore() and a pristine copy is
needed for the AbortTask.

+      metadata_.object_stores[object_store_id];
   leveldb::Status s =
       backing_store_->DeleteObjectStore(transaction->BackingStoreTransaction(),
                                         transaction->database()->id(),
-                                        object_store_metadata.id);
+                                        object_store_id);
   if (!s.ok()) {
     base::string16 error_string =
         ASCIIToUTF16("Internal error deleting object store '") +
         object_store_metadata.name + ASCIIToUTF16("'.");
     IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
                                  error_string);
     transaction->Abort(error);
     if (s.IsCorruption())
       factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
                                              error);
+    return;

[jsbell, 2014/05/19 22:07:08]

Ditto.

   }
+
+  RemoveObjectStore(object_store_id);
+  transaction->ScheduleAbortTask(
+      base::Bind(&IndexedDBDatabase::DeleteObjectStoreAbortOperation,
+                 this,
+                 object_store_metadata));
 }
 
 void IndexedDBDatabase::VersionChangeOperation(
     int64 version,
     scoped_refptr<IndexedDBCallbacks> callbacks,
     scoped_ptr<IndexedDBConnection> connection,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::VersionChangeOperation");
   int64 old_version = metadata_.int_version;
   DCHECK_GT(version, old_version);
-  metadata_.int_version = version;
+
   if (!backing_store_->UpdateIDBDatabaseIntVersion(
-          transaction->BackingStoreTransaction(),
-          id(),
-          metadata_.int_version)) {
+          transaction->BackingStoreTransaction(), id(), version)) {
     IndexedDBDatabaseError error(
         blink::WebIDBDatabaseExceptionUnknownError,
         ASCIIToUTF16(
             "Internal error writing data to stable storage when "
             "updating version."));
     callbacks->OnError(error);
     transaction->Abort(error);
     return;
   }
+
+  transaction->ScheduleAbortTask(
+      base::Bind(&IndexedDBDatabase::VersionChangeAbortOperation,
+                 this,
+                 metadata_.version,
+                 metadata_.int_version));
+  metadata_.int_version = version;
+  metadata_.version = kNoStringVersion;
+
   DCHECK(!pending_second_half_open_);
   pending_second_half_open_.reset(
       new PendingSuccessCall(callbacks, connection.get(), version));
   callbacks->OnUpgradeNeeded(old_version, connection.Pass(), metadata());
 }
 
 void IndexedDBDatabase::TransactionFinished(IndexedDBTransaction* transaction,
                                             bool committed) {
   DCHECK(transactions_.find(transaction->id()) != transactions_.end());
   DCHECK_EQ(transactions_[transaction->id()], transaction);
@@ skipping 262 matching lines @@
     scoped_ptr<IndexedDBConnection> connection,
     int64 transaction_id,
     int64 requested_version) {
 
   std::vector<int64> object_store_ids;
   CreateTransaction(transaction_id,
                     connection.get(),
                     object_store_ids,
                     indexed_db::TRANSACTION_VERSION_CHANGE);
 
-  transactions_[transaction_id]
-      ->ScheduleTask(base::Bind(&IndexedDBDatabase::VersionChangeOperation,
-                                this,
-                                requested_version,
-                                callbacks,
-                                base::Passed(&connection)),
-                     base::Bind(&IndexedDBDatabase::VersionChangeAbortOperation,
-                                this,
-                                metadata_.version,
-                                metadata_.int_version));
-
+  transactions_[transaction_id]->ScheduleTask(
+      base::Bind(&IndexedDBDatabase::VersionChangeOperation,
+                 this,
+                 requested_version,
+                 callbacks,
+                 base::Passed(&connection)));
   DCHECK(!pending_second_half_open_);
 }
 
 void IndexedDBDatabase::DeleteDatabase(
     scoped_refptr<IndexedDBCallbacks> callbacks) {
 
   if (IsDeleteDatabaseBlocked()) {
     for (ConnectionSet::const_iterator it = connections_.begin();
          it != connections_.end();
          ++it) {
@@ skipping 119 matching lines @@
     const base::string16& previous_version,
     int64 previous_int_version,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::VersionChangeAbortOperation");
   DCHECK(!transaction);
   metadata_.version = previous_version;
   metadata_.int_version = previous_int_version;
 }
 
 }  // namespace content