Changeset 241468 in webkit

Timestamp:

Feb 13, 2019, 1:32:44 PM (8 years ago)

Author:

sihui_liu@apple.com

Message:

REGRESSION: [ Mac Debug WK2 ] Layout Test storage/indexeddb/key-type-infinity-private.html is a flaky crash
​https://bugs.webkit.org/show_bug.cgi?id=194413
<rdar://problem/47897254>

Reviewed by Brady Eidson.

IDB clients expected transaction operations to be executed in order, but in
UniqueIDBDatabase::immediateCloseForUserDelete, callbacks in callback map were errored out randomly.
This patch added a callback queue to UniqueIDBDatabase to make sure callbacks will be called in the same order
as IDB Server receives the request.

• Modules/indexeddb/server/UniqueIDBDatabase.cpp:

(WebCore::IDBServer::UniqueIDBDatabase::storeCallbackOrFireError):
(WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
(WebCore::IDBServer::UniqueIDBDatabase::performErrorCallback):
(WebCore::IDBServer::UniqueIDBDatabase::performKeyDataCallback):
(WebCore::IDBServer::UniqueIDBDatabase::performGetResultCallback):
(WebCore::IDBServer::UniqueIDBDatabase::performGetAllResultsCallback):
(WebCore::IDBServer::UniqueIDBDatabase::performCountCallback):
(WebCore::IDBServer::UniqueIDBDatabase::forgetErrorCallback):

• Modules/indexeddb/server/UniqueIDBDatabase.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/indexeddb/server/UniqueIDBDatabase.cpp (modified) (12 diffs)
• Modules/indexeddb/server/UniqueIDBDatabase.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-02-13  Sihui Liu  <sihui_liu@apple.com>
+
+        REGRESSION: [ Mac Debug WK2 ] Layout Test storage/indexeddb/key-type-infinity-private.html is a flaky crash
+        https://bugs.webkit.org/show_bug.cgi?id=194413
+        <rdar://problem/47897254>
+
+        Reviewed by Brady Eidson.
+
+        IDB clients expected transaction operations to be executed in order, but in 
+        UniqueIDBDatabase::immediateCloseForUserDelete, callbacks in callback map were errored out randomly.
+        This patch added a callback queue to UniqueIDBDatabase to make sure callbacks will be called in the same order
+        as IDB Server receives the request.
+
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::storeCallbackOrFireError):
+        (WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
+        (WebCore::IDBServer::UniqueIDBDatabase::performErrorCallback):
+        (WebCore::IDBServer::UniqueIDBDatabase::performKeyDataCallback):
+        (WebCore::IDBServer::UniqueIDBDatabase::performGetResultCallback):
+        (WebCore::IDBServer::UniqueIDBDatabase::performGetAllResultsCallback):
+        (WebCore::IDBServer::UniqueIDBDatabase::performCountCallback):
+        (WebCore::IDBServer::UniqueIDBDatabase::forgetErrorCallback):
+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
+
 2019-02-13  John Wilander  <wilander@apple.com>
 

trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp

@@ -424 +424 @@
     ASSERT(!m_errorCallbacks.contains(identifier));
     m_errorCallbacks.add(identifier, WTFMove(callback));
+    m_callbackQueue.append(identifier);
     return identifier;
 }
@@ -437 +438 @@
     ASSERT(!m_keyDataCallbacks.contains(identifier));
     m_keyDataCallbacks.add(identifier, WTFMove(callback));
+    m_callbackQueue.append(identifier);
     return identifier;
 }
@@ -450 +452 @@
     ASSERT(!m_getResultCallbacks.contains(identifier));
     m_getResultCallbacks.add(identifier, WTFMove(callback));
+    m_callbackQueue.append(identifier);
     return identifier;
 }
@@ -463 +466 @@
     ASSERT(!m_getAllResultsCallbacks.contains(identifier));
     m_getAllResultsCallbacks.add(identifier, WTFMove(callback));
+    m_callbackQueue.append(identifier);
     return identifier;
 }
@@ -476 +480 @@
     ASSERT(!m_countCallbacks.contains(identifier));
     m_countCallbacks.add(identifier, WTFMove(callback));
+    m_callbackQueue.append(identifier);
     return identifier;
 }
@@ -1829 +1834 @@
     IDBKeyData keyData;
     IDBGetResult getResult;
-
-    for (auto identifier : copyToVector(m_errorCallbacks.keys()))
-        performErrorCallback(identifier, error);
-
-    for (auto identifier : copyToVector(m_keyDataCallbacks.keys()))
-        performKeyDataCallback(identifier, error, keyData);
-
-    for (auto identifier : copyToVector(m_getResultCallbacks.keys()))
-        performGetResultCallback(identifier, error, getResult);
-
-    for (auto identifier : copyToVector(m_countCallbacks.keys()))
-        performCountCallback(identifier, error, 0);
+    IDBGetAllResult getAllResult;
+
+    while (!m_callbackQueue.isEmpty()) {
+        auto identifier = m_callbackQueue.first();
+        if (m_errorCallbacks.contains(identifier))
+            performErrorCallback(identifier, error);
+        else if (m_keyDataCallbacks.contains(identifier))
+            performKeyDataCallback(identifier, error, keyData);
+        else if (m_getResultCallbacks.contains(identifier))
+            performGetResultCallback(identifier, error, getResult);
+        else if (m_countCallbacks.contains(identifier))
+            performCountCallback(identifier, error, 0);
+        else if (m_getAllResultsCallbacks.contains(identifier))
+            performGetAllResultsCallback(identifier, error, getAllResult);
+        else
+            ASSERT_NOT_REACHED();
+    }
 
     // Error out all IDBOpenDBRequests
@@ -1881 +1891 @@
     auto callback = m_errorCallbacks.take(callbackIdentifier);
     ASSERT(callback || m_hardClosedForUserDelete);
-    if (callback)
+    if (callback) {
         callback(error);
+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
+        m_callbackQueue.removeFirst();
+    }
 }
 
@@ -1889 +1902 @@
     auto callback = m_keyDataCallbacks.take(callbackIdentifier);
     ASSERT(callback || m_hardClosedForUserDelete);
-    if (callback)
+    if (callback) {
         callback(error, resultKey);
+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
+        m_callbackQueue.removeFirst();
+    }
 }
 
@@ -1897 +1913 @@
     auto callback = m_getResultCallbacks.take(callbackIdentifier);
     ASSERT(callback || m_hardClosedForUserDelete);
-    if (callback)
+    if (callback) {
         callback(error, resultData);
+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
+        m_callbackQueue.removeFirst();
+    }
 }
 
@@ -1905 +1924 @@
     auto callback = m_getAllResultsCallbacks.take(callbackIdentifier);
     ASSERT(callback || m_hardClosedForUserDelete);
-    if (callback)
+    if (callback) {
         callback(error, resultData);
+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
+        m_callbackQueue.removeFirst();
+    }
 }
 
@@ -1913 +1935 @@
     auto callback = m_countCallbacks.take(callbackIdentifier);
     ASSERT(callback || m_hardClosedForUserDelete);
-    if (callback)
+    if (callback) {
         callback(error, count);
+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
+        m_callbackQueue.removeFirst();
+    }
 }
 
@@ -1920 +1945 @@
 {
     ASSERT(m_errorCallbacks.contains(callbackIdentifier));
+    ASSERT(m_callbackQueue.last() == callbackIdentifier);
+    m_callbackQueue.removeLast();
     m_errorCallbacks.remove(callbackIdentifier);
 }

trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h

@@ -251 +251 @@
     HashMap<uint64_t, GetAllResultsCallback> m_getAllResultsCallbacks;
     HashMap<uint64_t, CountCallback> m_countCallbacks;
+    Deque<uint64_t> m_callbackQueue;
 
     Timer m_operationAndTransactionTimer;