- Revision
- 219298
- Author
- beid...@apple.com
- Date
- 2017-07-10 10:43:38 -0700 (Mon, 10 Jul 2017)
Log Message
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.
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.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (219297 => 219298)
--- trunk/Source/WTF/ChangeLog 2017-07-10 17:09:38 UTC (rev 219297)
+++ trunk/Source/WTF/ChangeLog 2017-07-10 17:43:38 UTC (rev 219298)
@@ -1,3 +1,19 @@
+2017-07-10 Brady Eidson <beid...@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....@gmail.com>
[JSC] Use fastMalloc / fastFree for STL containers
Modified: trunk/Source/WTF/wtf/CrossThreadQueue.h (219297 => 219298)
--- trunk/Source/WTF/wtf/CrossThreadQueue.h 2017-07-10 17:09:38 UTC (rev 219297)
+++ trunk/Source/WTF/wtf/CrossThreadQueue.h 2017-07-10 17:43:38 UTC (rev 219298)
@@ -46,12 +46,15 @@
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>
@@ -58,6 +61,7 @@
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 @@
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;
Modified: trunk/Source/WebCore/ChangeLog (219297 => 219298)
--- trunk/Source/WebCore/ChangeLog 2017-07-10 17:09:38 UTC (rev 219297)
+++ trunk/Source/WebCore/ChangeLog 2017-07-10 17:43:38 UTC (rev 219298)
@@ -1,3 +1,81 @@
+2017-07-10 Brady Eidson <beid...@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 <cdu...@apple.com>
Further WebResourceLoadStatisticsStore / ResourceLoadStatisticsStore clean up
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h (219297 => 219298)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h 2017-07-10 17:09:38 UTC (rev 219297)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h 2017-07-10 17:43:38 UTC (rev 219298)
@@ -29,6 +29,7 @@
#include "IDBDatabaseInfo.h"
#include "IDBError.h"
+#include <wtf/MainThread.h>
namespace WebCore {
@@ -64,7 +65,7 @@
class IDBBackingStore {
public:
- virtual ~IDBBackingStore() { }
+ virtual ~IDBBackingStore() { RELEASE_ASSERT(!isMainThread()); }
virtual IDBError getOrEstablishDatabaseInfo(IDBDatabaseInfo&) = 0;
@@ -98,6 +99,9 @@
virtual bool supportsSimultaneousTransactions() = 0;
virtual bool isEphemeral() = 0;
+
+protected:
+ IDBBackingStore() { RELEASE_ASSERT(!isMainThread()); }
};
} // namespace IDBServer
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp (219297 => 219298)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp 2017-07-10 17:09:38 UTC (rev 219297)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp 2017-07-10 17:43:38 UTC (rev 219298)
@@ -121,7 +121,7 @@
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 @@
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 @@
return;
}
- HashSet<RefPtr<UniqueIDBDatabase>> openDatabases;
+ HashSet<UniqueIDBDatabase*> openDatabases;
for (auto* connection : m_databaseConnections.values())
openDatabases.add(&connection->database());
@@ -561,7 +564,7 @@
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) {
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.h (219297 => 219298)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.h 2017-07-10 17:09:38 UTC (rev 219297)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBServer.h 2017-07-10 17:43:38 UTC (rev 219298)
@@ -98,7 +98,7 @@
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 @@
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;
Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp (219297 => 219298)
--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp 2017-07-10 17:09:38 UTC (rev 219297)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp 2017-07-10 17:43:38 UTC (rev 219298)
@@ -73,7 +73,10 @@
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 @@
{
LOG(IndexedDB, "UniqueIDBDatabase::openDatabaseConnection");
ASSERT(!m_hardClosedForUserDelete);
+ ASSERT(isMainThread());
m_pendingOpenDBRequests.add(ServerOpenDBRequest::create(connection, requestData));
@@ -260,15 +264,43 @@
ASSERT(!isMainThread());
LOG(IndexedDB, "(db) UniqueIDBDatabase::performUnconditionalDeleteBackingStore");
- if (!m_backingStore)
- return;
+ if (m_backingStore)
+ m_backingStore->deleteBackingStore();
- 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 = 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)
{
ASSERT(isMainThread());
@@ -278,6 +310,7 @@
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,7 +333,12 @@
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;
}
@@ -307,12 +345,6 @@
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 @@
ASSERT(!m_hardClosedForUserDelete);
ASSERT(m_currentOpenDBRequest);
- RefPtr<UniqueIDBDatabase> protectedThis(this);
-
if (m_currentOpenDBRequest->isOpenRequest())
performCurrentOpenOperation();
else if (m_currentOpenDBRequest->isDeleteRequest())
@@ -1235,11 +1265,9 @@
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::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 @@
{
LOG(IndexedDB, "(main) UniqueIDBDatabase::operationAndTransactionTimerFired");
ASSERT(!m_hardClosedForUserDelete);
+ ASSERT(isMainThread());
- RefPtr<UniqueIDBDatabase> protectedThis(this);
-
// 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::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 @@
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 @@
// 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::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,11 +1744,7 @@
{
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));
}
@@ -1730,30 +1751,23 @@
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,10 +1776,10 @@
void UniqueIDBDatabase::maybeFinishHardClose()
{
- if (m_hardCloseProtector && isDoneWithHardClose()) {
+ if (m_owningPointerForClose && isDoneWithHardClose()) {
callOnMainThread([this] {
ASSERT(isDoneWithHardClose());
- m_hardCloseProtector = nullptr;
+ m_owningPointerForClose = nullptr;
});
}
}
@@ -1772,7 +1786,7 @@
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 @@
{
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 @@
// 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)
Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h (219297 => 219298)
--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h 2017-07-10 17:09:38 UTC (rev 219297)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h 2017-07-10 17:43:38 UTC (rev 219298)
@@ -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 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 @@
bool hardClosedForUserDelete() const { return m_hardClosedForUserDelete; }
private:
- UniqueIDBDatabase(IDBServer&, const IDBDatabaseIdentifier&);
-
void handleDatabaseOperations();
void handleCurrentOperation();
void performCurrentOpenOperation();
@@ -144,6 +137,8 @@
void connectionClosedFromServer(UniqueIDBDatabaseConnection&);
+ void scheduleShutdownForClose();
+
// Database thread operations
void deleteBackingStore(const IDBDatabaseIdentifier&);
void openBackingStore(const IDBDatabaseIdentifier&);
@@ -169,6 +164,7 @@
void performActivateTransactionInBackingStore(uint64_t callbackIdentifier, const IDBTransactionInfo&);
void performUnconditionalDeleteBackingStore();
+ void shutdownForClose();
// Main thread callbacks
void didDeleteBackingStore(uint64_t deletedVersion);
@@ -191,6 +187,7 @@
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 @@
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
