Title: [201390] trunk/Source/WebCore
Revision
201390
Author
beid...@apple.com
Date
2016-05-25 11:20:26 -0700 (Wed, 25 May 2016)

Log Message

Modern IDB: IDB objects from a worker thread might be destroyed on the main thread.
https://bugs.webkit.org/show_bug.cgi?id=158004

Reviewed by Alex Christensen.

No new tests (Spuriously reproduces on the bots, but I've been unable to construct a reliable test).

* Modules/indexeddb/client/IDBConnectionProxy.cpp:
(WebCore::IDBClient::IDBConnectionProxy::completeOpenDBRequest):
(WebCore::IDBClient::IDBConnectionProxy::notifyOpenDBRequestBlocked):
(WebCore::IDBClient::IDBConnectionProxy::didCommitTransaction):
(WebCore::IDBClient::IDBConnectionProxy::didAbortTransaction):
(WebCore::IDBClient::IDBConnectionProxy::unregisterDatabaseConnection):
(WebCore::IDBClient::removeItemsMatchingCurrentThread):
(WebCore::IDBClient::IDBConnectionProxy::forgetActivityForCurrentThread): Clear out all objects that originated on this thread.
(WebCore::IDBClient::IDBConnectionProxy::takeIDBOpenDBRequest): Deleted.
* Modules/indexeddb/client/IDBConnectionProxy.h:

* workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::stopIndexedDatabase):
* workers/WorkerGlobalScope.h:

* workers/WorkerThread.cpp:
(WebCore::WorkerThread::stop):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (201389 => 201390)


--- trunk/Source/WebCore/ChangeLog	2016-05-25 18:02:13 UTC (rev 201389)
+++ trunk/Source/WebCore/ChangeLog	2016-05-25 18:20:26 UTC (rev 201390)
@@ -1,3 +1,30 @@
+2016-05-25  Brady Eidson  <beid...@apple.com>
+
+        Modern IDB: IDB objects from a worker thread might be destroyed on the main thread.
+        https://bugs.webkit.org/show_bug.cgi?id=158004
+
+        Reviewed by Alex Christensen.
+
+        No new tests (Spuriously reproduces on the bots, but I've been unable to construct a reliable test).
+
+        * Modules/indexeddb/client/IDBConnectionProxy.cpp:
+        (WebCore::IDBClient::IDBConnectionProxy::completeOpenDBRequest):
+        (WebCore::IDBClient::IDBConnectionProxy::notifyOpenDBRequestBlocked):
+        (WebCore::IDBClient::IDBConnectionProxy::didCommitTransaction):
+        (WebCore::IDBClient::IDBConnectionProxy::didAbortTransaction):
+        (WebCore::IDBClient::IDBConnectionProxy::unregisterDatabaseConnection):
+        (WebCore::IDBClient::removeItemsMatchingCurrentThread):
+        (WebCore::IDBClient::IDBConnectionProxy::forgetActivityForCurrentThread): Clear out all objects that originated on this thread.
+        (WebCore::IDBClient::IDBConnectionProxy::takeIDBOpenDBRequest): Deleted.
+        * Modules/indexeddb/client/IDBConnectionProxy.h:
+
+        * workers/WorkerGlobalScope.cpp:
+        (WebCore::WorkerGlobalScope::stopIndexedDatabase):
+        * workers/WorkerGlobalScope.h:
+
+        * workers/WorkerThread.cpp:
+        (WebCore::WorkerThread::stop):
+
 2016-05-25  Nael Ouedraogo  <nael.ouedra...@crf.canon.fr>
 
         Remove unused slotBase parameter in bindings generator

Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp (201389 => 201390)


--- trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp	2016-05-25 18:02:13 UTC (rev 201389)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp	2016-05-25 18:20:26 UTC (rev 201390)
@@ -100,18 +100,6 @@
     completeOpenDBRequest(resultData);
 }
 
-RefPtr<IDBOpenDBRequest> IDBConnectionProxy::takeIDBOpenDBRequest(IDBOpenDBRequest& request)
-{
-    ASSERT(request.originThreadID() == currentThread());
-
-    Locker<Lock> locker(m_openDBRequestMapLock);
-
-    auto mappedRequest = m_openDBRequestMap.take(request.resourceIdentifier());
-    ASSERT(mappedRequest.get() == &request);
-
-    return mappedRequest;
-}
-
 void IDBConnectionProxy::completeOpenDBRequest(const IDBResultData& resultData)
 {
     ASSERT(isMainThread());
@@ -120,10 +108,10 @@
     {
         Locker<Lock> locker(m_openDBRequestMapLock);
         request = m_openDBRequestMap.get(resultData.requestIdentifier());
-        ASSERT(request);
     }
 
-    ASSERT(request);
+    if (!request)
+        return;
 
     request->performCallbackOnOriginThread(*request, &IDBOpenDBRequest::requestCompleted, resultData);
 }
@@ -272,7 +260,8 @@
         request = m_openDBRequestMap.get(requestIdentifier);
     }
 
-    ASSERT(request);
+    if (!request)
+        return;
 
     request->performCallbackOnOriginThread(*request, &IDBOpenDBRequest::requestBlocked, oldVersion, newVersion);
 }
@@ -325,7 +314,8 @@
         transaction = m_committingTransactions.take(transactionIdentifier);
     }
 
-    ASSERT(transaction);
+    if (!transaction)
+        return;
 
     transaction->performCallbackOnOriginThread(*transaction, &IDBTransaction::didCommit, error);
 }
@@ -349,7 +339,8 @@
         transaction = m_abortingTransactions.take(transactionIdentifier);
     }
 
-    ASSERT(transaction);
+    if (!transaction)
+        return;
 
     transaction->performCallbackOnOriginThread(*transaction, &IDBTransaction::didAbort, error);
 }
@@ -440,8 +431,7 @@
 {
     Locker<Lock> locker(m_databaseConnectionMapLock);
 
-    ASSERT(m_databaseConnectionMap.contains(database.databaseConnectionIdentifier()));
-    ASSERT(m_databaseConnectionMap.get(database.databaseConnectionIdentifier()) == &database);
+    ASSERT(!m_databaseConnectionMap.contains(database.databaseConnectionIdentifier()) || m_databaseConnectionMap.get(database.databaseConnectionIdentifier()) == &database);
     m_databaseConnectionMap.remove(database.databaseConnectionIdentifier());
 }
 
@@ -453,6 +443,46 @@
         m_activeOperations.remove(operation->identifier());
 }
 
+template<typename KeyType, typename ValueType>
+void removeItemsMatchingCurrentThread(HashMap<KeyType, ValueType>& map)
+{
+    auto currentThreadID = currentThread();
+
+    Vector<KeyType> keys;
+    keys.reserveInitialCapacity(map.size());
+    for (auto& iterator : map) {
+        if (iterator.value->originThreadID() == currentThreadID)
+            keys.uncheckedAppend(iterator.key);
+    }
+
+    for (auto& key : keys)
+        map.remove(key);
+}
+
+void IDBConnectionProxy::forgetActivityForCurrentThread()
+{
+    ASSERT(!isMainThread());
+
+    {
+        Locker<Lock> lock(m_databaseConnectionMapLock);
+        removeItemsMatchingCurrentThread(m_databaseConnectionMap);
+    }
+    {
+        Locker<Lock> lock(m_openDBRequestMapLock);
+        removeItemsMatchingCurrentThread(m_openDBRequestMap);
+    }
+    {
+        Locker<Lock> lock(m_transactionMapLock);
+        removeItemsMatchingCurrentThread(m_pendingTransactions);
+        removeItemsMatchingCurrentThread(m_committingTransactions);
+        removeItemsMatchingCurrentThread(m_abortingTransactions);
+    }
+    {
+        Locker<Lock> lock(m_transactionOperationLock);
+        removeItemsMatchingCurrentThread(m_activeOperations);
+    }
+}
+
 } // namesapce IDBClient
 } // namespace WebCore
 

Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h (201389 => 201390)


--- trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h	2016-05-25 18:02:13 UTC (rev 201389)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h	2016-05-25 18:20:26 UTC (rev 201390)
@@ -110,9 +110,8 @@
     void registerDatabaseConnection(IDBDatabase&);
     void unregisterDatabaseConnection(IDBDatabase&);
 
-    RefPtr<IDBOpenDBRequest> takeIDBOpenDBRequest(IDBOpenDBRequest&);
-
     void forgetActiveOperations(const Vector<RefPtr<TransactionOperation>>&);
+    void forgetActivityForCurrentThread();
 
 private:
     void completeOpenDBRequest(const IDBResultData&);

Modified: trunk/Source/WebCore/workers/WorkerGlobalScope.cpp (201389 => 201390)


--- trunk/Source/WebCore/workers/WorkerGlobalScope.cpp	2016-05-25 18:02:13 UTC (rev 201389)
+++ trunk/Source/WebCore/workers/WorkerGlobalScope.cpp	2016-05-25 18:20:26 UTC (rev 201390)
@@ -132,6 +132,14 @@
     return nullptr;
 #endif
 }
+
+void WorkerGlobalScope::stopIndexedDatabase()
+{
+#if ENABLE(INDEXED_DATABASE_IN_WORKERS)
+    ASSERT(m_connectionProxy);
+    m_connectionProxy->forgetActivityForCurrentThread();
+#endif
+}
 #endif // ENABLE(INDEXED_DATABASE)
 
 WorkerLocation& WorkerGlobalScope::location() const

Modified: trunk/Source/WebCore/workers/WorkerGlobalScope.h (201389 => 201390)


--- trunk/Source/WebCore/workers/WorkerGlobalScope.h	2016-05-25 18:02:13 UTC (rev 201389)
+++ trunk/Source/WebCore/workers/WorkerGlobalScope.h	2016-05-25 18:20:26 UTC (rev 201390)
@@ -76,6 +76,7 @@
 
 #if ENABLE(INDEXED_DATABASE)
     IDBClient::IDBConnectionProxy* idbConnectionProxy() final;
+    void stopIndexedDatabase();
 #endif
 
     bool shouldBypassMainWorldContentSecurityPolicy() const final { return m_shouldBypassMainWorldContentSecurityPolicy; }

Modified: trunk/Source/WebCore/workers/WorkerThread.cpp (201389 => 201390)


--- trunk/Source/WebCore/workers/WorkerThread.cpp	2016-05-25 18:02:13 UTC (rev 201389)
+++ trunk/Source/WebCore/workers/WorkerThread.cpp	2016-05-25 18:20:26 UTC (rev 201390)
@@ -208,6 +208,10 @@
         m_runLoop.postTaskAndTerminate({ ScriptExecutionContext::Task::CleanupTask, [] (ScriptExecutionContext& context ) {
             WorkerGlobalScope& workerGlobalScope = downcast<WorkerGlobalScope>(context);
 
+#if ENABLE(INDEXED_DATABASE)
+            workerGlobalScope.stopIndexedDatabase();
+#endif
+
             workerGlobalScope.stopActiveDOMObjects();
             workerGlobalScope.notifyObserversOfStop();
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to