Changeset 242555 in webkit

Timestamp:

Mar 6, 2019 11:20:53 AM (5 years ago)

Author:

sihui_liu@apple.com

Message:

Assertion Failed: m_databaseQueue.isKilled() in UniqueIDBDatabase::~UniqueIDBDatabase()
​https://bugs.webkit.org/show_bug.cgi?id=195073
<rdar://problem/48285200>

Reviewed by Geoffrey Garen.

r240931 removed a retain cycle between IDBConnectionToServer and IDBConnectionToServerDelegate, so
IDBConnectionToServerDelegate, or InProcessIDBServer would not live forever. When IDBDatabase is gone,
InProcessIDBServer would schedule a notifification to IDBServer with databaseConnectionClosed. IDBServer would
then notify UniqueIDBDatabase. When UniqueIDBDatabase finds all database connections are gone, it would acquires
its only reference pointer from IDBServer schedule and perform a shutdown that kills its database task queue.

The assertion failure tells us UniqueIDBDatabase was destructed at when IDBServer was destructed, which means
UniqueIDBDatabase had not acquired its pointer. It's probably because UniqueIDBDatabase had unfinished tasks or
the operation timer function had not been executed. Since UniqueIDBDatabase needs to complete shutdown process,
we should make IDBServer live as long as UniqueIDBDatabase by keeping a reference pointer of IDBServer in
UniqueIDBDatabase.

• Modules/indexeddb/server/UniqueIDBDatabase.cpp:

(WebCore::IDBServer::UniqueIDBDatabase::UniqueIDBDatabase):
(WebCore::IDBServer::UniqueIDBDatabase::deleteBackingStore):
(WebCore::IDBServer::UniqueIDBDatabase::scheduleShutdownForClose):
(WebCore::IDBServer::UniqueIDBDatabase::openBackingStore):
(WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTask):
(WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTaskReply):
(WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
(WebCore::IDBServer::UniqueIDBDatabase::notifyServerAboutClose):

• Modules/indexeddb/server/UniqueIDBDatabase.h:

(WebCore::IDBServer::UniqueIDBDatabase::server):

Location:

trunk/Source/WebCore

Files:

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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-03-06  Sihui Liu  <sihui_liu@apple.com>
+
+        Assertion Failed: m_databaseQueue.isKilled() in UniqueIDBDatabase::~UniqueIDBDatabase()
+        https://bugs.webkit.org/show_bug.cgi?id=195073
+        <rdar://problem/48285200>
+
+        Reviewed by Geoffrey Garen.
+
+        r240931 removed a retain cycle between IDBConnectionToServer and IDBConnectionToServerDelegate, so 
+        IDBConnectionToServerDelegate, or InProcessIDBServer would not live forever. When IDBDatabase is gone, 
+        InProcessIDBServer would schedule a notifification to IDBServer with databaseConnectionClosed. IDBServer would 
+        then notify UniqueIDBDatabase. When UniqueIDBDatabase finds all database connections are gone, it would acquires
+        its only reference pointer from IDBServer schedule and perform a shutdown that kills its database task queue.
+
+        The assertion failure tells us UniqueIDBDatabase was destructed at when IDBServer was destructed, which means 
+        UniqueIDBDatabase had not acquired its pointer. It's probably because UniqueIDBDatabase had unfinished tasks or
+        the operation timer function had not been executed. Since UniqueIDBDatabase needs to complete shutdown process,
+        we should make IDBServer live as long as UniqueIDBDatabase by keeping a reference pointer of IDBServer in 
+        UniqueIDBDatabase. 
+
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::UniqueIDBDatabase):
+        (WebCore::IDBServer::UniqueIDBDatabase::deleteBackingStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::scheduleShutdownForClose):
+        (WebCore::IDBServer::UniqueIDBDatabase::openBackingStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTask):
+        (WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTaskReply):
+        (WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
+        (WebCore::IDBServer::UniqueIDBDatabase::notifyServerAboutClose):
+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
+        (WebCore::IDBServer::UniqueIDBDatabase::server):
+
 2019-03-06  Rob Buis  <rbuis@igalia.com>
 

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

@@ -56 +56 @@
 
 UniqueIDBDatabase::UniqueIDBDatabase(IDBServer& server, const IDBDatabaseIdentifier& identifier)
-    : m_server(server)
+    : m_server(&server)
     , m_identifier(identifier)
     , m_operationAndTransactionTimer(*this, &UniqueIDBDatabase::operationAndTransactionTimerFired)
@@ -246 +246 @@
         m_backingStoreIsEphemeral = false;
     } else {
-        auto backingStore = m_server.createBackingStore(identifier);
+        auto backingStore = m_server->createBackingStore(identifier);
 
         IDBDatabaseInfo databaseInfo;
@@ -278 +278 @@
 
     RELEASE_ASSERT(!m_owningPointerForClose);
-    m_owningPointerForClose = m_server.closeAndTakeUniqueIDBDatabase(*this);
+    m_owningPointerForClose = m_server->closeAndTakeUniqueIDBDatabase(*this);
 
     notifyServerAboutClose(CloseState::Start);
@@ -637 +637 @@
 
     ASSERT(!m_backingStore);
-    m_backingStore = m_server.createBackingStore(identifier);
+    m_backingStore = m_server->createBackingStore(identifier);
     m_backingStoreSupportsSimultaneousTransactions = m_backingStore->supportsSimultaneousTransactions();
     m_backingStoreIsEphemeral = m_backingStore->isEphemeral();
@@ -1764 +1764 @@
 {
     m_databaseQueue.append(WTFMove(task));
-    m_server.postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::executeNextDatabaseTask));
+    m_server->postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::executeNextDatabaseTask));
 }
 
@@ -1770 +1770 @@
 {
     m_databaseReplyQueue.append(WTFMove(task));
-    m_server.postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::executeNextDatabaseTaskReply));
+    m_server->postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::executeNextDatabaseTaskReply));
 }
 
@@ -1899 +1899 @@
     // Otherwise, this database is still potentially active.
     // So we'll have it own itself and then perform a clean unconditional delete on the background thread.
-    m_owningPointerForClose = m_server.closeAndTakeUniqueIDBDatabase(*this);
+    m_owningPointerForClose = m_server->closeAndTakeUniqueIDBDatabase(*this);
     postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performUnconditionalDeleteBackingStore));
 }
@@ -1977 +1977 @@
 #if PLATFORM(IOS_FAMILY)
     if (state == CloseState::Start) 
-        m_server.closeDatabase(this);
+        m_server->closeDatabase(this);
     else
-        m_server.didCloseDatabase(this);
+        m_server->didCloseDatabase(this);
 #else
     UNUSED_PARAM(state);

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

@@ -83 +83 @@
 
     const IDBDatabaseInfo& info() const;
-    IDBServer& server() { return m_server; }
+    IDBServer& server() { return *m_server; }
     const IDBDatabaseIdentifier& identifier() const { return m_identifier; }
 
@@ -230 +230 @@
     void notifyServerAboutClose(CloseState);
 
-    IDBServer& m_server;
+    RefPtr<IDBServer> m_server;
     IDBDatabaseIdentifier m_identifier;