Don't allow writers to starve

dom/indexedDB/IDBObjectStoreRequest.cpp

@@ -384,6 +384,8 @@ IDBObjectStoreRequest::GetName(nsAString& aName)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   aName.Assign(mName);
   return NS_OK;
 }
@@ -393,6 +395,8 @@ IDBObjectStoreRequest::GetKeyPath(nsAString& aKeyPath)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   aKeyPath.Assign(mKeyPath);
   return NS_OK;
 }
@@ -402,6 +406,8 @@ IDBObjectStoreRequest::GetIndexNames(nsIDOMDOMStringList** aIndexNames)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   nsRefPtr<nsDOMStringList> list(new nsDOMStringList());
 #if 0
   PRUint32 count = mIndexes.Length();
@@ -419,6 +425,8 @@ IDBObjectStoreRequest::Get(nsIVariant* aKey,
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   Key key;
   nsresult rv = GetKeyFromVariant(aKey, key);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -446,6 +454,9 @@ IDBObjectStoreRequest::GetAll(nsIIDBKeyRange* aKeyRange,
                               nsIIDBRequest** _retval)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   NS_NOTYETIMPLEMENTED("Implement me!");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
@@ -457,6 +468,8 @@ IDBObjectStoreRequest::Add(nsIVariant* /* aValue */,
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   if (mMode != nsIIDBTransaction::READ_WRITE) {
     return NS_ERROR_OBJECT_IS_IMMUTABLE;
   }
@@ -493,6 +506,8 @@ IDBObjectStoreRequest::Modify(nsIVariant* /* aValue */,
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   if (mMode != nsIIDBTransaction::READ_WRITE) {
     return NS_ERROR_OBJECT_IS_IMMUTABLE;
   }
@@ -529,6 +544,8 @@ IDBObjectStoreRequest::AddOrModify(nsIVariant* /* aValue */,
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   if (mMode != nsIIDBTransaction::READ_WRITE) {
     return NS_ERROR_OBJECT_IS_IMMUTABLE;
   }
@@ -564,6 +581,8 @@ IDBObjectStoreRequest::Remove(nsIVariant* aKey,
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   if (mMode != nsIIDBTransaction::READ_WRITE) {
     return NS_ERROR_OBJECT_IS_IMMUTABLE;
   }
@@ -598,6 +617,10 @@ IDBObjectStoreRequest::OpenCursor(nsIIDBKeyRange* aRange,
                                   nsIIDBRequest** _retval)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
+  NS_NOTYETIMPLEMENTED("Implement me!");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
@@ -608,6 +631,9 @@ IDBObjectStoreRequest::CreateIndex(const nsAString& aName,
                                    nsIIDBRequest** _retval)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   NS_NOTYETIMPLEMENTED("Implement me!");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
@@ -617,6 +643,9 @@ IDBObjectStoreRequest::Index(const nsAString& aName,
                              nsIIDBIndexRequest** _retval)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   NS_NOTYETIMPLEMENTED("Implement me!");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
@@ -626,6 +655,9 @@ IDBObjectStoreRequest::RemoveIndex(const nsAString& aName,
                                    nsIIDBRequest** _retval)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(mTransaction->TransactionIsOpen());
+
   NS_NOTYETIMPLEMENTED("Implement me!");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
@@ -638,14 +670,19 @@ AddHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
   nsresult rv;
 
   bool mayOverwrite = mOverwrite;
+  bool unsetKey = mKey.IsUnset();
 
-  if (mKey.IsUnset()) {
+  if (unsetKey) {
     NS_ASSERTION(mAutoIncrement, "Must have a key for non-autoIncrement!");
 
     // Will need to add first and then set the key later.
     mayOverwrite = false;
   }
 
+  if (mAutoIncrement && !unsetKey) {
+    mayOverwrite = true;
+  }
+
   nsCOMPtr<mozIStorageStatement> stmt =
     mTransaction->AddStatement(mCreate, mayOverwrite, mAutoIncrement);
   NS_ENSURE_TRUE(stmt, nsIIDBDatabaseException::UNKNOWN_ERR);
@@ -678,8 +715,6 @@ AddHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
   }
 
   if (mAutoIncrement && mCreate && !mOverwrite) {
-    bool unsetKey = mKey.IsUnset();
-
 #ifdef DEBUG
     PRInt64 oldKey = unsetKey ? 0 : mKey.IntValue();
 #endif

dom/indexedDB/IDBTransactionRequest.cpp

@@ -441,6 +441,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::GetDb(nsIIDBDatabase** aDB)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   NS_ADDREF(*aDB = mDatabase);
   return NS_OK;
 }
@@ -449,6 +452,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::GetReadyState(PRUint16* aReadyState)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   *aReadyState = mReadyState;
   return NS_OK;
 }
@@ -457,6 +463,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::GetMode(PRUint16* aMode)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   *aMode = mMode;
   return NS_OK;
 }
@@ -466,6 +475,8 @@ IDBTransactionRequest::GetObjectStoreNames(nsIDOMDOMStringList** aObjectStores)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   nsRefPtr<nsDOMStringList> list(new nsDOMStringList());
   PRUint32 count = mObjectStoreNames.Length();
   for (PRUint32 index = 0; index < count; index++) {
@@ -481,6 +492,8 @@ IDBTransactionRequest::ObjectStore(const nsAString& aName,
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   ObjectStoreInfo* info = nsnull;
 
   PRUint32 count = mObjectStoreNames.Length();
@@ -512,6 +525,8 @@ IDBTransactionRequest::Abort()
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   NS_WARNING("Abort doesn't actually do anything yet! Fix me now!");
 
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   nsCOMPtr<nsIRunnable> runnable =
     IDBEvent::CreateGenericEventRunnable(NS_LITERAL_STRING(ABORT_EVT_STR),
                                          this);
@@ -527,6 +542,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::GetOncomplete(nsIDOMEventListener** aOncomplete)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   return GetInnerEventListener(mOnCompleteListener, aOncomplete);
 }
 
@@ -534,6 +552,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::SetOncomplete(nsIDOMEventListener* aOncomplete)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   return RemoveAddEventListener(NS_LITERAL_STRING(COMPLETE_EVT_STR),
                                 mOnCompleteListener, aOncomplete);
 }
@@ -542,6 +563,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::GetOnabort(nsIDOMEventListener** aOnabort)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   return GetInnerEventListener(mOnAbortListener, aOnabort);
 }
 
@@ -549,6 +573,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::SetOnabort(nsIDOMEventListener* aOnabort)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   return RemoveAddEventListener(NS_LITERAL_STRING(ABORT_EVT_STR),
                                 mOnAbortListener, aOnabort);
 }
@@ -557,6 +584,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::GetOntimeout(nsIDOMEventListener** aOntimeout)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   return GetInnerEventListener(mOnTimeoutListener, aOntimeout);
 }
 
@@ -564,6 +594,9 @@ NS_IMETHODIMP
 IDBTransactionRequest::SetOntimeout(nsIDOMEventListener* aOntimeout)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+
+  NS_ENSURE_STATE(TransactionIsOpen());
+
   return RemoveAddEventListener(NS_LITERAL_STRING(TIMEOUT_EVT_STR),
                                 mOnTimeoutListener, aOntimeout);
 }

dom/indexedDB/IDBTransactionRequest.h

@@ -103,6 +103,11 @@ public:
 
   void CloseConnection();
 
+  bool TransactionIsOpen() {
+    return mReadyState == nsIIDBTransaction::INITIAL ||
+           mReadyState == nsIIDBTransaction::LOADING;
+  }
+
 private:
   IDBTransactionRequest();
   ~IDBTransactionRequest();

dom/indexedDB/IndexedDatabaseRequest.cpp

@@ -151,9 +151,10 @@ CreateTables(mozIStorageConnection* aDBConn)
   // Table `ai_object_data`
   rv = aDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
     "CREATE TABLE ai_object_data ("
-      "id INTEGER PRIMARY KEY, "
+      "id INTEGER, "
       "object_store_id INTEGER NOT NULL, "
       "data TEXT NOT NULL, "
+      "PRIMARY KEY (id), "
       "FOREIGN KEY (object_store_id) REFERENCES object_store(id) ON DELETE "
         "CASCADE"
     ");"

dom/indexedDB/TransactionThreadPool.cpp

@@ -67,10 +67,11 @@ bool gShutdown = false;
 
 struct TransactionObjectStoreInfo
 {
-  TransactionObjectStoreInfo() : writing(false) { }
+  TransactionObjectStoreInfo() : writing(false), writerWaiting(false) { }
 
   nsString objectStoreName;
   bool writing;
+  bool writerWaiting;
 };
 
 } // anonymous namespace
@@ -311,7 +312,7 @@ TransactionThreadPool::TransactionCanRun(IDBTransactionRequest* aTransaction,
   for (PRUint32 transactionIndex = 0;
        transactionIndex < transactionCount;
        transactionIndex++) {
-    const TransactionInfo& transactionInfo =
+    TransactionInfo& transactionInfo =
       transactionsInProgress->ElementAt(transactionIndex);
 
     if (transactionInfo.transaction == aTransaction) {
@@ -321,14 +322,14 @@ TransactionThreadPool::TransactionCanRun(IDBTransactionRequest* aTransaction,
     }
 
     // Not our transaction, see if the objectStores overlap.
-    const nsTArray<TransactionObjectStoreInfo>& objectStoreInfoArray =
+    nsTArray<TransactionObjectStoreInfo>& objectStoreInfoArray =
       transactionInfo.objectStoreInfo;
 
     PRUint32 objectStoreCount = objectStoreInfoArray.Length();
     for (PRUint32 objectStoreIndex = 0;
          objectStoreIndex < objectStoreCount;
          objectStoreIndex++) {
-      const TransactionObjectStoreInfo& objectStoreInfo =
+      TransactionObjectStoreInfo& objectStoreInfo =
         objectStoreInfoArray[objectStoreIndex];
 
       if (objectStoreNames.Contains(objectStoreInfo.objectStoreName)) {
@@ -336,12 +337,13 @@ TransactionThreadPool::TransactionCanRun(IDBTransactionRequest* aTransaction,
         switch (mode) {
           case nsIIDBTransaction::READ_WRITE: {
             // Someone else is reading or writing to this table, we can't
-            // run now.
+            // run now. Mark that we're waiting for it though.
+            objectStoreInfo.writerWaiting = true;
             return false;
           }
 
           case nsIIDBTransaction::READ_ONLY: {
-            if (objectStoreInfo.writing) {
+            if (objectStoreInfo.writing || objectStoreInfo.writerWaiting) {
               // Someone else is writing to this table, we can't run now.
               return false;
             }

dom/indexedDB/test/Makefile.in

@@ -62,6 +62,7 @@ _TEST_FILES = \
   test_readonly_transactions.html \
   test_remove_objectStore.html \
   test_setVersion.html \
+  test_writer_starvation.html \
   $(NULL)
 
 libs:: $(_TEST_FILES)

dom/indexedDB/test/test_key_requirements.html

@@ -53,6 +53,15 @@
 
       is(event.result, key2, "modify gave the same key back");
 
+      key2 = 100;
+
+      request = objectStore.add({}, key2);
+      request.onerror = errorHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+      event = yield;
+
+      is(event.result, key2, "modify gave the same key back");
+
       try {
         objectStore.addOrModify({});
         ok(false, "addOrModify with no key should throw!");

dom/indexedDB/test/test_writer_starvation.html

@@ -0,0 +1,106 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <title>Indexed Database Property Test</title>
+
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+
+  <script type="text/javascript;version=1.7">
+    function testSteps()
+    {
+      const READ_ONLY = Components.interfaces.nsIIDBTransaction.READ_ONLY;
+      const READ_WRITE = Components.interfaces.nsIIDBTransaction.READ_WRITE;
+
+      const name = window.location.pathname;
+      const description = "My Test Database";
+
+      let request = indexedDB.open(name, description);
+      request.onerror = errorHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+      let event = yield;
+
+      let db = event.result;
+
+      request = db.createObjectStore("foo", "", true);
+      request.onerror = errorHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+      let event = yield;
+
+      is(event.transaction.mode, READ_WRITE, "Correct mode");
+
+      request = event.result.add({});
+      request.onerror = errorHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+      event = yield;
+
+      let key = event.result;
+      ok(key, "Got a key");
+
+      SimpleTest.executeSoon(function() { testGenerator.next(); });
+      yield;
+
+      let objectStore = db.objectStore("foo");
+
+      let continueReading = true;
+      let readerCount = 0;
+      let callbackCount = 0;
+      let finalCallbackCount = 0;
+
+      // Generate a bunch of reads right away without returning to the event
+      // loop.
+      for (let i = 0; i < 20; i++) {
+        readerCount++;
+        request = objectStore.get(key);
+        request.onerror = errorHandler;
+        request.onsuccess = function(event) {
+          callbackCount++;
+        };
+      }
+
+      while (continueReading) {
+        readerCount++;
+        request = db.objectStore("foo").get(key);
+        request.onerror = errorHandler;
+        request.onsuccess = function(event) {
+          is(event.transaction.mode, READ_ONLY, "Correct mode");
+          callbackCount++;
+          if (callbackCount == 100) {
+            request = db.objectStore("foo", READ_WRITE).add({}, readerCount);
+            request.onerror = errorHandler;
+            request.onsuccess = function(event) {
+              continueReading = false;
+              finalCallbackCount = callbackCount;
+              ok(event.result == callbackCount,
+                 "write callback came before later reads");
+            }
+          }
+        };
+
+        SimpleTest.executeSoon(function() { testGenerator.next(); });
+        yield;
+      }
+
+      while (callbackCount < readerCount) {
+        SimpleTest.executeSoon(function() { testGenerator.next(); });
+        yield;
+      }
+
+      is(callbackCount, readerCount, "All requests accounted for");
+      ok(callbackCount > finalCallbackCount, "More readers after writer");
+
+      finishTest();
+      yield;
+    }
+  </script>
+  <script type="text/javascript;version=1.7" src="helpers.js"></script>
+
+</head>
+
+<body onload="runTest();"></body>
+
+</html>