Cleanup lifetime issues of UniqueIDBDatabase and IDBBackingStore.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Jul 2017 17:43:38 +0000 (17:43 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Jul 2017 17:43:38 +0000 (17:43 +0000)
<rdar://problem/32908525> and https://bugs.webkit.org/show_bug.cgi?id=174244

Reviewed by David Kilzer and Alex Christensen.

Source/WebCore:

No targeted test possible, implicitly covered by all IDB tests.

The original idea behind UniqueIDBDatabase lifetime was that they are ThreadSafeRefCounted and
we take protector Refs when any operation that needs it alive is in flight.

This added variability to their lifetime which made it difficult to enforce a few different
design invariants, namely:
    - UniqueIBDDatabase objects are always created and destroyed only on the main thread.
    - IDBBackingStore objects are always created and destroyed only on the database thread.

This patch removes the ref counting and instead ties UniqueIDBDatabase lifetime to a
std::unique_ptr that is owned by the IDBServer.

Whenever any operations on the UniqueIDBDatabase are in flight it is kept alive by virtue
of that unique_ptr in the IDBServer. Once a UniqueIDBDatabase is completely done with all of
its work, the following happens:
    - On the main thread the IDBServer removes the unique_ptr owning the UniqueIDBDatabase
      from its map.
    - It hands the unique_ptr to the UniqueIDBDatabase itself, which schedules one final
      database thread task.
    - That database thread task is to destroy the IDBBackingStore, kill its message queues,
      and then message back to the main thread for one final task.
    - That main thread task is to release the unique_ptr, resulting in destruction of the
      UniqueIDBDatabase object.

This is safe, predictable, solves the lifetime issues that r218516 originally tried to solve,
and solves the lifetime issues that r218516 introduced.

(This patch also adds many more assertions to cover various design invariants throughout the
lifecycle of a particular UniqueIDBDatabase)

ASSERT that IDBBackingStores are only ever created and destroyed on the background thread:
* Modules/indexeddb/server/IDBBackingStore.h:
(WebCore::IDBServer::IDBBackingStore::~IDBBackingStore):
(WebCore::IDBServer::IDBBackingStore::IDBBackingStore):

Transition UniqueIDBDatabase ownership from a RefPtr to a std::unique_ptr:
* Modules/indexeddb/server/IDBServer.cpp:
(WebCore::IDBServer::IDBServer::getOrCreateUniqueIDBDatabase):
(WebCore::IDBServer::IDBServer::closeAndTakeUniqueIDBDatabase):
(WebCore::IDBServer::IDBServer::closeAndDeleteDatabasesModifiedSince):
(WebCore::IDBServer::IDBServer::closeAndDeleteDatabasesForOrigins):
(WebCore::IDBServer::IDBServer::closeUniqueIDBDatabase): Deleted.
* Modules/indexeddb/server/IDBServer.h:

Make all the other changes mentioned above:
* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::~UniqueIDBDatabase): Bulk up on ASSERTs
(WebCore::IDBServer::UniqueIDBDatabase::openDatabaseConnection):
(WebCore::IDBServer::UniqueIDBDatabase::performUnconditionalDeleteBackingStore):
(WebCore::IDBServer::UniqueIDBDatabase::scheduleShutdownForClose):
(WebCore::IDBServer::UniqueIDBDatabase::shutdownForClose):
(WebCore::IDBServer::UniqueIDBDatabase::didShutdownForClose):
(WebCore::IDBServer::UniqueIDBDatabase::didDeleteBackingStore):
(WebCore::IDBServer::UniqueIDBDatabase::handleCurrentOperation):
(WebCore::IDBServer::UniqueIDBDatabase::performIterateCursor):
(WebCore::IDBServer::UniqueIDBDatabase::performPrefetchCursor):
(WebCore::IDBServer::UniqueIDBDatabase::operationAndTransactionTimerFired):
(WebCore::IDBServer::UniqueIDBDatabase::activateTransactionInBackingStore):
(WebCore::IDBServer::UniqueIDBDatabase::transactionCompleted):
(WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTask):
(WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTaskReply):
(WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTask):
(WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTaskReply):
(WebCore::IDBServer::UniqueIDBDatabase::maybeFinishHardClose):
(WebCore::IDBServer::UniqueIDBDatabase::isDoneWithHardClose):
(WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
(WebCore::IDBServer::UniqueIDBDatabase::didPerformUnconditionalDeleteBackingStore): Deleted.
* Modules/indexeddb/server/UniqueIDBDatabase.h:
(WebCore::IDBServer::UniqueIDBDatabase::create): Deleted.

Source/WTF:

Add proper "kill" support to CrossThreadQueue, as well as isEmpty() support.

* wtf/CrossThreadQueue.h:
(WTF::CrossThreadQueue<DataType>::append):
(WTF::CrossThreadQueue<DataType>::kill):
(WTF::CrossThreadQueue<DataType>::isKilled):
(WTF::CrossThreadQueue<DataType>::isEmpty):
(WTF::CrossThreadQueue::isKilled): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219298 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/CrossThreadQueue.h
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h
Source/WebCore/Modules/indexeddb/server/IDBServer.cpp
Source/WebCore/Modules/indexeddb/server/IDBServer.h
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h

index 6ff5909..855b9d7 100644 (file)
@@ -1,3 +1,19 @@
+2017-07-10  Brady Eidson  <beidson@apple.com>
+
+        Cleanup lifetime issues of UniqueIDBDatabase and IDBBackingStore.
+        <rdar://problem/32908525> and https://bugs.webkit.org/show_bug.cgi?id=174244
+
+        Reviewed by David Kilzer and Alex Christensen. 
+
+        Add proper "kill" support to CrossThreadQueue, as well as isEmpty() support.
+
+        * wtf/CrossThreadQueue.h:
+        (WTF::CrossThreadQueue<DataType>::append):
+        (WTF::CrossThreadQueue<DataType>::kill):
+        (WTF::CrossThreadQueue<DataType>::isKilled):
+        (WTF::CrossThreadQueue<DataType>::isEmpty):
+        (WTF::CrossThreadQueue::isKilled): Deleted.
+
 2017-07-09  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] Use fastMalloc / fastFree for STL containers
index 9681bec..ade4d90 100644 (file)
@@ -46,18 +46,22 @@ public:
     DataType waitForMessage();
     std::optional<DataType> tryGetMessage();
 
-    bool isKilled() const { return false; }
+    void kill();
+    bool isKilled() const;
+    bool isEmpty() const;
 
 private:
     mutable Lock m_lock;
     Condition m_condition;
     Deque<DataType> m_queue;
+    bool m_killed { false };
 };
 
 template<typename DataType>
 void CrossThreadQueue<DataType>::append(DataType&& message)
 {
     LockHolder lock(m_lock);
+    ASSERT(!m_killed);
     m_queue.append(WTFMove(message));
     m_condition.notifyOne();
 }
@@ -90,6 +94,28 @@ std::optional<DataType> CrossThreadQueue<DataType>::tryGetMessage()
     return m_queue.takeFirst();
 }
 
+template<typename DataType>
+void CrossThreadQueue<DataType>::kill()
+{
+    LockHolder lock(m_lock);
+    m_killed = true;
+    m_condition.notifyAll();
+}
+
+template<typename DataType>
+bool CrossThreadQueue<DataType>::isKilled() const
+{
+    LockHolder lock(m_lock);
+    return m_killed;
+}
+
+template<typename DataType>
+bool CrossThreadQueue<DataType>::isEmpty() const
+{
+    LockHolder lock(m_lock);
+    return m_queue.isEmpty();
+}
+
 } // namespace WTF
 
 using WTF::CrossThreadQueue;
index e4069f4..f0efe77 100644 (file)
@@ -1,3 +1,81 @@
+2017-07-10  Brady Eidson  <beidson@apple.com>
+
+        Cleanup lifetime issues of UniqueIDBDatabase and IDBBackingStore.
+        <rdar://problem/32908525> and https://bugs.webkit.org/show_bug.cgi?id=174244
+
+        Reviewed by David Kilzer and Alex Christensen. 
+
+        No targeted test possible, implicitly covered by all IDB tests.
+
+        The original idea behind UniqueIDBDatabase lifetime was that they are ThreadSafeRefCounted and
+        we take protector Refs when any operation that needs it alive is in flight.
+        
+        This added variability to their lifetime which made it difficult to enforce a few different 
+        design invariants, namely:
+            - UniqueIBDDatabase objects are always created and destroyed only on the main thread.
+            - IDBBackingStore objects are always created and destroyed only on the database thread.
+        
+        This patch removes the ref counting and instead ties UniqueIDBDatabase lifetime to a
+        std::unique_ptr that is owned by the IDBServer.
+        
+        Whenever any operations on the UniqueIDBDatabase are in flight it is kept alive by virtue
+        of that unique_ptr in the IDBServer. Once a UniqueIDBDatabase is completely done with all of
+        its work, the following happens:
+            - On the main thread the IDBServer removes the unique_ptr owning the UniqueIDBDatabase
+              from its map.
+            - It hands the unique_ptr to the UniqueIDBDatabase itself, which schedules one final 
+              database thread task.
+            - That database thread task is to destroy the IDBBackingStore, kill its message queues,
+              and then message back to the main thread for one final task.
+            - That main thread task is to release the unique_ptr, resulting in destruction of the
+              UniqueIDBDatabase object.
+        
+        This is safe, predictable, solves the lifetime issues that r218516 originally tried to solve,
+        and solves the lifetime issues that r218516 introduced.
+
+        (This patch also adds many more assertions to cover various design invariants throughout the
+        lifecycle of a particular UniqueIDBDatabase)
+
+        ASSERT that IDBBackingStores are only ever created and destroyed on the background thread:
+        * Modules/indexeddb/server/IDBBackingStore.h:
+        (WebCore::IDBServer::IDBBackingStore::~IDBBackingStore):
+        (WebCore::IDBServer::IDBBackingStore::IDBBackingStore):
+        
+        Transition UniqueIDBDatabase ownership from a RefPtr to a std::unique_ptr:
+        * Modules/indexeddb/server/IDBServer.cpp:
+        (WebCore::IDBServer::IDBServer::getOrCreateUniqueIDBDatabase):
+        (WebCore::IDBServer::IDBServer::closeAndTakeUniqueIDBDatabase):
+        (WebCore::IDBServer::IDBServer::closeAndDeleteDatabasesModifiedSince):
+        (WebCore::IDBServer::IDBServer::closeAndDeleteDatabasesForOrigins):
+        (WebCore::IDBServer::IDBServer::closeUniqueIDBDatabase): Deleted.
+        * Modules/indexeddb/server/IDBServer.h:
+        
+        Make all the other changes mentioned above:
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::~UniqueIDBDatabase): Bulk up on ASSERTs
+        (WebCore::IDBServer::UniqueIDBDatabase::openDatabaseConnection): 
+        (WebCore::IDBServer::UniqueIDBDatabase::performUnconditionalDeleteBackingStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::scheduleShutdownForClose):
+        (WebCore::IDBServer::UniqueIDBDatabase::shutdownForClose):
+        (WebCore::IDBServer::UniqueIDBDatabase::didShutdownForClose):
+        (WebCore::IDBServer::UniqueIDBDatabase::didDeleteBackingStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::handleCurrentOperation):
+        (WebCore::IDBServer::UniqueIDBDatabase::performIterateCursor):
+        (WebCore::IDBServer::UniqueIDBDatabase::performPrefetchCursor):
+        (WebCore::IDBServer::UniqueIDBDatabase::operationAndTransactionTimerFired):
+        (WebCore::IDBServer::UniqueIDBDatabase::activateTransactionInBackingStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::transactionCompleted):
+        (WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTask):
+        (WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTaskReply):
+        (WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTask):
+        (WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTaskReply):
+        (WebCore::IDBServer::UniqueIDBDatabase::maybeFinishHardClose):
+        (WebCore::IDBServer::UniqueIDBDatabase::isDoneWithHardClose):
+        (WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
+        (WebCore::IDBServer::UniqueIDBDatabase::didPerformUnconditionalDeleteBackingStore): Deleted.
+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
+        (WebCore::IDBServer::UniqueIDBDatabase::create): Deleted.
+
 2017-07-10  Chris Dumez  <cdumez@apple.com>
 
         Further WebResourceLoadStatisticsStore / ResourceLoadStatisticsStore clean up
index 9dde74c..f88efa0 100644 (file)
@@ -29,6 +29,7 @@
 
 #include "IDBDatabaseInfo.h"
 #include "IDBError.h"
+#include <wtf/MainThread.h>
 
 namespace WebCore {
 
@@ -64,7 +65,7 @@ public:
 
 class IDBBackingStore {
 public:
-    virtual ~IDBBackingStore() { }
+    virtual ~IDBBackingStore() { RELEASE_ASSERT(!isMainThread()); }
 
     virtual IDBError getOrEstablishDatabaseInfo(IDBDatabaseInfo&) = 0;
 
@@ -98,6 +99,9 @@ public:
 
     virtual bool supportsSimultaneousTransactions() = 0;
     virtual bool isEphemeral() = 0;
+
+protected:
+    IDBBackingStore() { RELEASE_ASSERT(!isMainThread()); }
 };
 
 } // namespace IDBServer
index 420c3a6..7af04a3 100644 (file)
@@ -121,7 +121,7 @@ UniqueIDBDatabase& IDBServer::getOrCreateUniqueIDBDatabase(const IDBDatabaseIden
 
     auto uniqueIDBDatabase = m_uniqueIDBDatabaseMap.add(identifier, nullptr);
     if (uniqueIDBDatabase.isNewEntry)
-        uniqueIDBDatabase.iterator->value = UniqueIDBDatabase::create(*this, identifier);
+        uniqueIDBDatabase.iterator->value = std::make_unique<UniqueIDBDatabase>(*this, identifier);
 
     return *uniqueIDBDatabase.iterator->value;
 }
@@ -171,12 +171,15 @@ void IDBServer::deleteDatabase(const IDBRequestData& requestData)
     database->handleDelete(*connection, requestData);
 }
 
-void IDBServer::closeUniqueIDBDatabase(UniqueIDBDatabase& database)
+std::unique_ptr<UniqueIDBDatabase> IDBServer::closeAndTakeUniqueIDBDatabase(UniqueIDBDatabase& database)
 {
     LOG(IndexedDB, "IDBServer::closeUniqueIDBDatabase");
     ASSERT(isMainThread());
 
-    m_uniqueIDBDatabaseMap.remove(database.identifier());
+    auto uniquePointer = m_uniqueIDBDatabaseMap.take(database.identifier());
+    ASSERT(uniquePointer);
+
+    return uniquePointer;
 }
 
 void IDBServer::abortTransaction(const IDBResourceIdentifier& transactionIdentifier)
@@ -545,7 +548,7 @@ void IDBServer::closeAndDeleteDatabasesModifiedSince(std::chrono::system_clock::
         return;
     }
 
-    HashSet<RefPtr<UniqueIDBDatabase>> openDatabases;
+    HashSet<UniqueIDBDatabase*> openDatabases;
     for (auto* connection : m_databaseConnections.values())
         openDatabases.add(&connection->database());
 
@@ -561,7 +564,7 @@ void IDBServer::closeAndDeleteDatabasesForOrigins(const Vector<SecurityOriginDat
     auto addResult = m_deleteDatabaseCompletionHandlers.add(callbackID, WTFMove(completionHandler));
     ASSERT_UNUSED(addResult, addResult.isNewEntry);
 
-    HashSet<RefPtr<UniqueIDBDatabase>> openDatabases;
+    HashSet<UniqueIDBDatabase*> openDatabases;
     for (auto* connection : m_databaseConnections.values()) {
         const auto& identifier = connection->database().identifier();
         for (auto& origin : origins) {
index db10d69..924e202 100644 (file)
@@ -98,7 +98,7 @@ public:
     void registerTransaction(UniqueIDBDatabaseTransaction&);
     void unregisterTransaction(UniqueIDBDatabaseTransaction&);
 
-    void closeUniqueIDBDatabase(UniqueIDBDatabase&);
+    std::unique_ptr<UniqueIDBDatabase> closeAndTakeUniqueIDBDatabase(UniqueIDBDatabase&);
 
     std::unique_ptr<IDBBackingStore> createBackingStore(const IDBDatabaseIdentifier&);
 
@@ -122,7 +122,7 @@ private:
     void handleTaskRepliesOnMainThread();
 
     HashMap<uint64_t, RefPtr<IDBConnectionToClient>> m_connectionMap;
-    HashMap<IDBDatabaseIdentifier, RefPtr<UniqueIDBDatabase>> m_uniqueIDBDatabaseMap;
+    HashMap<IDBDatabaseIdentifier, std::unique_ptr<UniqueIDBDatabase>> m_uniqueIDBDatabaseMap;
 
     RefPtr<Thread> m_thread { nullptr };
     Lock m_databaseThreadCreationLock;
index ab0d2ae..e89ac39 100644 (file)
@@ -73,7 +73,10 @@ UniqueIDBDatabase::~UniqueIDBDatabase()
     ASSERT(m_openDatabaseConnections.isEmpty());
     ASSERT(m_clientClosePendingDatabaseConnections.isEmpty());
     ASSERT(m_serverClosePendingDatabaseConnections.isEmpty());
-    ASSERT(!m_queuedTaskCount);
+
+    RELEASE_ASSERT(m_databaseQueue.isKilled());
+    RELEASE_ASSERT(m_databaseReplyQueue.isKilled());
+    RELEASE_ASSERT(!m_backingStore);
 }
 
 const IDBDatabaseInfo& UniqueIDBDatabase::info() const
@@ -86,6 +89,7 @@ void UniqueIDBDatabase::openDatabaseConnection(IDBConnectionToClient& connection
 {
     LOG(IndexedDB, "UniqueIDBDatabase::openDatabaseConnection");
     ASSERT(!m_hardClosedForUserDelete);
+    ASSERT(isMainThread());
 
     m_pendingOpenDBRequests.add(ServerOpenDBRequest::create(connection, requestData));
 
@@ -260,13 +264,41 @@ void UniqueIDBDatabase::performUnconditionalDeleteBackingStore()
     ASSERT(!isMainThread());
     LOG(IndexedDB, "(db) UniqueIDBDatabase::performUnconditionalDeleteBackingStore");
 
-    if (!m_backingStore)
-        return;
+    if (m_backingStore)
+        m_backingStore->deleteBackingStore();
+
+    shutdownForClose();
+}
+
+void UniqueIDBDatabase::scheduleShutdownForClose()
+{
+    ASSERT(isMainThread());
+
+    m_owningPointerForClose = m_server.closeAndTakeUniqueIDBDatabase(*this);
+    postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::shutdownForClose));
+}
+
+void UniqueIDBDatabase::shutdownForClose()
+{
+    ASSERT(!isMainThread());
+    ASSERT(m_owningPointerForClose.get() == this);
+
+    LOG(IndexedDB, "(db) UniqueIDBDatabase::shutdownForClose");
 
-    m_backingStore->deleteBackingStore();
     m_backingStore = nullptr;
     m_backingStoreSupportsSimultaneousTransactions = false;
     m_backingStoreIsEphemeral = false;
+
+    ASSERT(m_databaseQueue.isEmpty());
+    m_databaseQueue.kill();
+
+    postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didShutdownForClose));
+}
+
+void UniqueIDBDatabase::didShutdownForClose()
+{
+    ASSERT(m_databaseReplyQueue.isEmpty());
+    m_databaseReplyQueue.kill();
 }
 
 void UniqueIDBDatabase::didDeleteBackingStore(uint64_t deletedVersion)
@@ -278,6 +310,7 @@ void UniqueIDBDatabase::didDeleteBackingStore(uint64_t deletedVersion)
     ASSERT(!hasUnfinishedTransactions());
     ASSERT(m_pendingTransactions.isEmpty());
     ASSERT(m_openDatabaseConnections.isEmpty());
+    ASSERT(!m_backingStore);
 
     // It's possible that the openDBRequest was cancelled from client-side after the delete was already dispatched to the backingstore.
     // So it's okay if we don't have a currentOpenDBRequest, but if we do it has to be a deleteRequest.
@@ -300,19 +333,18 @@ void UniqueIDBDatabase::didDeleteBackingStore(uint64_t deletedVersion)
     m_deleteBackingStoreInProgress = false;
 
     if (m_clientClosePendingDatabaseConnections.isEmpty() && m_pendingOpenDBRequests.isEmpty()) {
-        m_server.closeUniqueIDBDatabase(*this);
+        // This UniqueIDBDatabase is now ready to be deleted.
+        ASSERT(m_databaseQueue.isEmpty());
+        ASSERT(m_databaseReplyQueue.isEmpty());
+        m_databaseQueue.kill();
+        m_databaseReplyQueue.kill();
+        callOnMainThread([owningRef = m_server.closeAndTakeUniqueIDBDatabase(*this)]{ });
         return;
     }
 
     invokeOperationAndTransactionTimer();
 }
 
-void UniqueIDBDatabase::didPerformUnconditionalDeleteBackingStore()
-{
-    // This function is a placeholder so the database thread can message back to the main thread.
-    ASSERT(m_hardClosedForUserDelete);
-}
-
 void UniqueIDBDatabase::handleDatabaseOperations()
 {
     ASSERT(isMainThread());
@@ -349,8 +381,6 @@ void UniqueIDBDatabase::handleCurrentOperation()
     ASSERT(!m_hardClosedForUserDelete);
     ASSERT(m_currentOpenDBRequest);
 
-    RefPtr<UniqueIDBDatabase> protectedThis(this);
-
     if (m_currentOpenDBRequest->isOpenRequest())
         performCurrentOpenOperation();
     else if (m_currentOpenDBRequest->isDeleteRequest())
@@ -1235,11 +1265,9 @@ void UniqueIDBDatabase::performIterateCursor(uint64_t callbackIdentifier, const
     IDBError error = m_backingStore->iterateCursor(transactionIdentifier, cursorIdentifier, data, result);
 
     if (error.isNull()) {
-        auto addResult = m_prefetchProtectors.add(cursorIdentifier, nullptr);
-        if (addResult.isNewEntry) {
-            addResult.iterator->value = this;
+        auto addResult = m_cursorPrefetches.add(cursorIdentifier);
+        if (addResult.isNewEntry)
             postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performPrefetchCursor, transactionIdentifier, cursorIdentifier));
-        }
     }
 
     postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didPerformIterateCursor, callbackIdentifier, error, result));
@@ -1248,13 +1276,13 @@ void UniqueIDBDatabase::performIterateCursor(uint64_t callbackIdentifier, const
 void UniqueIDBDatabase::performPrefetchCursor(const IDBResourceIdentifier& transactionIdentifier, const IDBResourceIdentifier& cursorIdentifier)
 {
     ASSERT(!isMainThread());
-    ASSERT(m_prefetchProtectors.contains(cursorIdentifier));
+    ASSERT(m_cursorPrefetches.contains(cursorIdentifier));
     LOG(IndexedDB, "(db) UniqueIDBDatabase::performPrefetchCursor");
 
     if (m_backingStore->prefetchCursor(transactionIdentifier, cursorIdentifier))
         postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performPrefetchCursor, transactionIdentifier, cursorIdentifier));
     else
-        postDatabaseTaskReply(WTF::Function<void ()>([prefetchProtector = m_prefetchProtectors.take(cursorIdentifier)]() { }));
+        m_cursorPrefetches.remove(cursorIdentifier);
 }
 
 void UniqueIDBDatabase::didPerformIterateCursor(uint64_t callbackIdentifier, const IDBError& error, const IDBGetResult& result)
@@ -1523,15 +1551,15 @@ void UniqueIDBDatabase::operationAndTransactionTimerFired()
 {
     LOG(IndexedDB, "(main) UniqueIDBDatabase::operationAndTransactionTimerFired");
     ASSERT(!m_hardClosedForUserDelete);
-
-    RefPtr<UniqueIDBDatabase> protectedThis(this);
+    ASSERT(isMainThread());
 
     // This UniqueIDBDatabase might be no longer in use by any web page.
     // Assuming it is not ephemeral, the server should now close it to free up resources.
     if (!m_backingStoreIsEphemeral && !isCurrentlyInUse()) {
         ASSERT(m_pendingTransactions.isEmpty());
         ASSERT(!hasUnfinishedTransactions());
-        m_server.closeUniqueIDBDatabase(*this);
+
+        scheduleShutdownForClose();
         return;
     }
 
@@ -1567,11 +1595,11 @@ void UniqueIDBDatabase::operationAndTransactionTimerFired()
 void UniqueIDBDatabase::activateTransactionInBackingStore(UniqueIDBDatabaseTransaction& transaction)
 {
     LOG(IndexedDB, "(main) UniqueIDBDatabase::activateTransactionInBackingStore");
+    ASSERT(isMainThread());
 
-    RefPtr<UniqueIDBDatabase> protectedThis(this);
     RefPtr<UniqueIDBDatabaseTransaction> refTransaction(&transaction);
 
-    ErrorCallback callback = [protectedThis, refTransaction](const IDBError& error) {
+    ErrorCallback callback = [refTransaction](const IDBError& error) {
         refTransaction->didActivateInBackingStore(error);
     };
 
@@ -1676,6 +1704,7 @@ void UniqueIDBDatabase::transactionCompleted(RefPtr<UniqueIDBDatabaseTransaction
     ASSERT(transaction);
     ASSERT(!m_inProgressTransactions.contains(transaction->info().identifier()));
     ASSERT(!m_finishingTransactions.contains(transaction->info().identifier()));
+    ASSERT(isMainThread());
 
     for (auto objectStore : transaction->objectStoreIdentifiers()) {
         if (!transaction->isReadOnly()) {
@@ -1694,7 +1723,7 @@ void UniqueIDBDatabase::transactionCompleted(RefPtr<UniqueIDBDatabaseTransaction
     // It's possible that this database had its backing store deleted but there were a few outstanding asynchronous operations.
     // If this transaction completing was the last of those operations, we can finally delete this UniqueIDBDatabase.
     if (m_clientClosePendingDatabaseConnections.isEmpty() && m_pendingOpenDBRequests.isEmpty() && !m_databaseInfo) {
-        m_server.closeUniqueIDBDatabase(*this);
+        scheduleShutdownForClose();
         return;
     }
 
@@ -1707,11 +1736,7 @@ void UniqueIDBDatabase::transactionCompleted(RefPtr<UniqueIDBDatabaseTransaction
 
 void UniqueIDBDatabase::postDatabaseTask(CrossThreadTask&& task)
 {
-    m_databaseQueue.append([protectedThis = makeRef(*this), task = WTFMove(task)]() mutable {
-        task.performTask();
-    });
-    ++m_queuedTaskCount;
-
+    m_databaseQueue.append(WTFMove(task));
     m_server.postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::executeNextDatabaseTask));
 }
 
@@ -1719,41 +1744,30 @@ void UniqueIDBDatabase::postDatabaseTaskReply(CrossThreadTask&& task)
 {
     ASSERT(!isMainThread());
 
-    m_databaseReplyQueue.append([protectedThis = makeRef(*this), task = WTFMove(task)]() mutable {
-        task.performTask();
-    });
-    ++m_queuedTaskCount;
-
+    m_databaseReplyQueue.append(WTFMove(task));
     m_server.postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::executeNextDatabaseTaskReply));
 }
 
 void UniqueIDBDatabase::executeNextDatabaseTask()
 {
     ASSERT(!isMainThread());
-    ASSERT(m_queuedTaskCount);
+    ASSERT(!m_databaseQueue.isKilled());
 
     auto task = m_databaseQueue.tryGetMessage();
     ASSERT(task);
 
-    (*task)();
-    --m_queuedTaskCount;
-
-    // Release the task on the main thread in case it holds the last reference to this,
-    // as UniqueIDBDatabase objects must be deleted on the main thread.
-    callOnMainThread([task = WTFMove(task)] {
-    });
+    task->performTask();
 }
 
 void UniqueIDBDatabase::executeNextDatabaseTaskReply()
 {
     ASSERT(isMainThread());
-    ASSERT(m_queuedTaskCount);
+    ASSERT(!m_databaseReplyQueue.isKilled());
 
     auto task = m_databaseReplyQueue.tryGetMessage();
     ASSERT(task);
 
-    (*task)();
-    --m_queuedTaskCount;
+    task->performTask();
 
     // If this database was force closed (e.g. for a user delete) and there are no more
     // cleanup tasks left, delete this.
@@ -1762,17 +1776,17 @@ void UniqueIDBDatabase::executeNextDatabaseTaskReply()
 
 void UniqueIDBDatabase::maybeFinishHardClose()
 {
-    if (m_hardCloseProtector && isDoneWithHardClose()) {
+    if (m_owningPointerForClose && isDoneWithHardClose()) {
         callOnMainThread([this] {
             ASSERT(isDoneWithHardClose());
-            m_hardCloseProtector = nullptr;
+            m_owningPointerForClose = nullptr;
         });
     }
 }
 
 bool UniqueIDBDatabase::isDoneWithHardClose()
 {
-    return !m_queuedTaskCount && m_clientClosePendingDatabaseConnections.isEmpty() && m_serverClosePendingDatabaseConnections.isEmpty();
+    return m_databaseQueue.isKilled() && m_clientClosePendingDatabaseConnections.isEmpty() && m_serverClosePendingDatabaseConnections.isEmpty();
 }
 
 static void errorOpenDBRequestForUserDelete(ServerOpenDBRequest& request)
@@ -1788,6 +1802,8 @@ void UniqueIDBDatabase::immediateCloseForUserDelete()
 {
     LOG(IndexedDB, "UniqueIDBDatabase::immediateCloseForUserDelete - Cancelling (%i, %i, %i, %i) callbacks", m_errorCallbacks.size(), m_keyDataCallbacks.size(), m_getResultCallbacks.size(), m_countCallbacks.size());
 
+    ASSERT(isMainThread());
+
     // Error out all transactions
     Vector<IDBResourceIdentifier> inProgressIdentifiers;
     copyKeysToVector(m_inProgressTransactions, inProgressIdentifiers);
@@ -1847,14 +1863,13 @@ void UniqueIDBDatabase::immediateCloseForUserDelete()
     // Set up the database to remain alive-but-inert until all of its background activity finishes and all
     // database connections confirm that they have closed.
     m_hardClosedForUserDelete = true;
-    m_hardCloseProtector = this;
+
+    // Remove this UniqueIDBDatabase from the IDBServer's set of open databases, and let it own itself.
+    // It will dispatch back to the main thread later to finalize deleting itself.
+    m_owningPointerForClose = m_server.closeAndTakeUniqueIDBDatabase(*this);
 
     // Have the database unconditionally delete itself on the database task queue.
     postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performUnconditionalDeleteBackingStore));
-
-    // Remove the database from the IDBServer's set of open databases.
-    // If there is no in-progress background thread activity for this database, it will be deleted here.
-    m_server.closeUniqueIDBDatabase(*this);
 }
 
 void UniqueIDBDatabase::performErrorCallback(uint64_t callbackIdentifier, const IDBError& error)
index e6fa565..a949490 100644 (file)
@@ -42,8 +42,6 @@
 #include <wtf/HashCountedSet.h>
 #include <wtf/HashSet.h>
 #include <wtf/ListHashSet.h>
-#include <wtf/Ref.h>
-#include <wtf/ThreadSafeRefCounted.h>
 
 namespace JSC {
 class ExecState;
@@ -74,13 +72,10 @@ typedef Function<void(const IDBError&, const IDBGetResult&)> GetResultCallback;
 typedef Function<void(const IDBError&, const IDBGetAllResult&)> GetAllResultsCallback;
 typedef Function<void(const IDBError&, uint64_t)> CountCallback;
 
-class UniqueIDBDatabase : public ThreadSafeRefCounted<UniqueIDBDatabase> {
+class UniqueIDBDatabase {
 public:
-    static Ref<UniqueIDBDatabase> create(IDBServer& server, const IDBDatabaseIdentifier& identifier)
-    {
-        return adoptRef(*new UniqueIDBDatabase(server, identifier));
-    }
-
+    UniqueIDBDatabase(IDBServer&, const IDBDatabaseIdentifier&);
+    UniqueIDBDatabase(UniqueIDBDatabase&) = delete;
     WEBCORE_EXPORT ~UniqueIDBDatabase();
 
     void openDatabaseConnection(IDBConnectionToClient&, const IDBRequestData&);
@@ -124,8 +119,6 @@ public:
     bool hardClosedForUserDelete() const { return m_hardClosedForUserDelete; }
 
 private:
-    UniqueIDBDatabase(IDBServer&, const IDBDatabaseIdentifier&);
-    
     void handleDatabaseOperations();
     void handleCurrentOperation();
     void performCurrentOpenOperation();
@@ -144,6 +137,8 @@ private:
 
     void connectionClosedFromServer(UniqueIDBDatabaseConnection&);
 
+    void scheduleShutdownForClose();
+
     // Database thread operations
     void deleteBackingStore(const IDBDatabaseIdentifier&);
     void openBackingStore(const IDBDatabaseIdentifier&);
@@ -169,6 +164,7 @@ private:
 
     void performActivateTransactionInBackingStore(uint64_t callbackIdentifier, const IDBTransactionInfo&);
     void performUnconditionalDeleteBackingStore();
+    void shutdownForClose();
 
     // Main thread callbacks
     void didDeleteBackingStore(uint64_t deletedVersion);
@@ -191,6 +187,7 @@ private:
     void didPerformAbortTransaction(uint64_t callbackIdentifier, const IDBError&, const IDBResourceIdentifier& transactionIdentifier);
     void didPerformActivateTransactionInBackingStore(uint64_t callbackIdentifier, const IDBError&);
     void didPerformUnconditionalDeleteBackingStore();
+    void didShutdownForClose();
 
     uint64_t storeCallbackOrFireError(ErrorCallback&&);
     uint64_t storeCallbackOrFireError(KeyDataCallback&&);
@@ -265,14 +262,13 @@ private:
 
     bool m_deleteBackingStoreInProgress { false };
 
-    CrossThreadQueue<Function<void ()>> m_databaseQueue;
-    CrossThreadQueue<Function<void ()>> m_databaseReplyQueue;
-    std::atomic<uint64_t> m_queuedTaskCount { 0 };
+    CrossThreadQueue<CrossThreadTask> m_databaseQueue;
+    CrossThreadQueue<CrossThreadTask> m_databaseReplyQueue;
 
     bool m_hardClosedForUserDelete { false };
-    RefPtr<UniqueIDBDatabase> m_hardCloseProtector;
+    std::unique_ptr<UniqueIDBDatabase> m_owningPointerForClose;
 
-    HashMap<IDBResourceIdentifier, RefPtr<UniqueIDBDatabase>> m_prefetchProtectors;
+    HashSet<IDBResourceIdentifier> m_cursorPrefetches;
 };
 
 } // namespace IDBServer