Changeset 241967 in webkit

Timestamp:

Feb 22, 2019 3:41:16 PM (5 years ago)

Author:

sihui_liu@apple.com

Message:

Crash under IDBServer::IDBConnectionToClient::identifier() const
​https://bugs.webkit.org/show_bug.cgi?id=194843
<rdar://problem/48203102>

Reviewed by Geoffrey Garen.

UniqueIDBDatabase should ignore requests from connections that are already closed.

Tests are hard to create without some tricks on UniqueIDBDatabase so this fix is verified manually.
One test is created by adding delay to UniqueIDBDatabase::openBackingStore on the background thread to make sure
disconnection of web process happens before UniqueIDBDatabase::didOpenBackingStore, because didOpenBackingStore
may start a version change transaction and ask for identifier from the connection that is already gone.

• Modules/indexeddb/server/IDBConnectionToClient.cpp:

(WebCore::IDBServer::IDBConnectionToClient::connectionToClientClosed):

• Modules/indexeddb/server/IDBConnectionToClient.h:

(WebCore::IDBServer::IDBConnectionToClient::isClosed):

• Modules/indexeddb/server/UniqueIDBDatabase.cpp:

(WebCore::IDBServer::UniqueIDBDatabase::clearStalePendingOpenDBRequests):
(WebCore::IDBServer::UniqueIDBDatabase::handleDatabaseOperations):
(WebCore::IDBServer::UniqueIDBDatabase::operationAndTransactionTimerFired):

• Modules/indexeddb/server/UniqueIDBDatabase.h:

Location:

trunk/Source/WebCore

Files:

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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-02-22  Sihui Liu  <sihui_liu@apple.com>
+
+        Crash under IDBServer::IDBConnectionToClient::identifier() const
+        https://bugs.webkit.org/show_bug.cgi?id=194843
+        <rdar://problem/48203102>
+
+        Reviewed by Geoffrey Garen.
+
+        UniqueIDBDatabase should ignore requests from connections that are already closed.
+
+        Tests are hard to create without some tricks on UniqueIDBDatabase so this fix is verified manually. 
+        One test is created by adding delay to UniqueIDBDatabase::openBackingStore on the background thread to make sure
+        disconnection of web process happens before UniqueIDBDatabase::didOpenBackingStore, because didOpenBackingStore
+        may start a version change transaction and ask for identifier from the connection that is already gone.
+
+        * Modules/indexeddb/server/IDBConnectionToClient.cpp:
+        (WebCore::IDBServer::IDBConnectionToClient::connectionToClientClosed):
+        * Modules/indexeddb/server/IDBConnectionToClient.h:
+        (WebCore::IDBServer::IDBConnectionToClient::isClosed):
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::clearStalePendingOpenDBRequests):
+        (WebCore::IDBServer::UniqueIDBDatabase::handleDatabaseOperations):
+        (WebCore::IDBServer::UniqueIDBDatabase::operationAndTransactionTimerFired):
+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
+
 2019-02-22  Wenson Hsieh  <wenson_hsieh@apple.com>
 

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

@@ -208 +208 @@
     }
 
+    m_isClosed = true;
     m_databaseConnections.clear();
 }

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

@@ -80 +80 @@
     void unregisterDatabaseConnection(UniqueIDBDatabaseConnection&);
     void connectionToClientClosed();
-
+    bool isClosed() { return m_isClosed; }
 private:
     IDBConnectionToClient(IDBConnectionToClientDelegate&);
@@ -86 +86 @@
     WeakPtr<IDBConnectionToClientDelegate> m_delegate;
     HashSet<UniqueIDBDatabaseConnection*> m_databaseConnections;
+    bool m_isClosed { false };
 };
 

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

@@ -345 +345 @@
 }
 
+void UniqueIDBDatabase::clearStalePendingOpenDBRequests()
+{
+    while (!m_pendingOpenDBRequests.isEmpty() && m_pendingOpenDBRequests.first()->connection().isClosed())
+        m_pendingOpenDBRequests.removeFirst();
+}
+
 void UniqueIDBDatabase::handleDatabaseOperations()
 {
@@ -354 +360 @@
         return;
 
-    if (m_versionChangeDatabaseConnection || m_versionChangeTransaction || m_currentOpenDBRequest) {
+    clearStalePendingOpenDBRequests();
+
+    if (m_versionChangeDatabaseConnection || m_versionChangeTransaction || (m_currentOpenDBRequest && !m_currentOpenDBRequest->connection().isClosed())) {
         // We can't start any new open-database operations right now, but we might be able to start handling a delete operation.
         if (!m_currentOpenDBRequest && !m_pendingOpenDBRequests.isEmpty() && m_pendingOpenDBRequests.first()->isDeleteRequest())
@@ -366 +374 @@
     }
 
-    if (m_pendingOpenDBRequests.isEmpty())
-        return;
+    if (m_pendingOpenDBRequests.isEmpty()) {
+        m_currentOpenDBRequest = nullptr;
+        return;
+    }
 
     m_currentOpenDBRequest = m_pendingOpenDBRequests.takeFirst();
@@ -1576 +1586 @@
     // The current operation might require multiple attempts to handle, so try to
     // make further progress on it now.
-    if (m_currentOpenDBRequest)
+    if (m_currentOpenDBRequest && !m_currentOpenDBRequest->connection().isClosed())
         handleCurrentOperation();
-
-    if (!m_currentOpenDBRequest)
+    else
         handleDatabaseOperations();
 

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

@@ -215 +215 @@
 
     bool prepareToFinishTransaction(UniqueIDBDatabaseTransaction&);
+    
+    void clearStalePendingOpenDBRequests();
 
     void postDatabaseTask(CrossThreadTask&&);