Title: [218516] trunk/Source/WebCore
- Revision
- 218516
- Author
- beid...@apple.com
- Date
- 2017-06-19 16:19:11 -0700 (Mon, 19 Jun 2017)
Log Message
Various IndexedDB crashes as an after effect of previous test.
<rdar://problem/31418761> and https://bugs.webkit.org/show_bug.cgi?id=170436
Reviewed by Chris Dumez.
No new test (No consistent test possible, in practice covered by all existing IDB tests)
This is timing related, where a UniqueIDBDatabase can be destroyed on the main thread while
it still has one task left to try to execute on the IDBServer thread.
The background thread tasks don't Ref<> the UniqueIDBDatabase, so even though task execution
took a Ref<> protector, there was still a small window for a race.
Should be closed up by making the background thread tasks themselves protect this.
* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTask):
(WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTaskReply):
(WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTask):
(WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTaskReply):
* Modules/indexeddb/server/UniqueIDBDatabase.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (218515 => 218516)
--- trunk/Source/WebCore/ChangeLog 2017-06-19 23:08:53 UTC (rev 218515)
+++ trunk/Source/WebCore/ChangeLog 2017-06-19 23:19:11 UTC (rev 218516)
@@ -1,3 +1,27 @@
+2017-06-19 Brady Eidson <beid...@apple.com>
+
+ Various IndexedDB crashes as an after effect of previous test.
+ <rdar://problem/31418761> and https://bugs.webkit.org/show_bug.cgi?id=170436
+
+ Reviewed by Chris Dumez.
+
+ No new test (No consistent test possible, in practice covered by all existing IDB tests)
+
+ This is timing related, where a UniqueIDBDatabase can be destroyed on the main thread while
+ it still has one task left to try to execute on the IDBServer thread.
+
+ The background thread tasks don't Ref<> the UniqueIDBDatabase, so even though task execution
+ took a Ref<> protector, there was still a small window for a race.
+
+ Should be closed up by making the background thread tasks themselves protect this.
+
+ * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+ (WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTask):
+ (WebCore::IDBServer::UniqueIDBDatabase::postDatabaseTaskReply):
+ (WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTask):
+ (WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTaskReply):
+ * Modules/indexeddb/server/UniqueIDBDatabase.h:
+
2017-06-19 Sam Weinig <s...@webkit.org>
[WebIDL] Add support for serializers that have members that are themselves serializers (or inherit being a serializer from a parent)
Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp (218515 => 218516)
--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp 2017-06-19 23:08:53 UTC (rev 218515)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp 2017-06-19 23:19:11 UTC (rev 218516)
@@ -1706,7 +1706,9 @@
void UniqueIDBDatabase::postDatabaseTask(CrossThreadTask&& task)
{
- m_databaseQueue.append(WTFMove(task));
+ m_databaseQueue.append([protectedThis = makeRef(*this), task = WTFMove(task)]() mutable {
+ task.performTask();
+ });
++m_queuedTaskCount;
m_server.postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::executeNextDatabaseTask));
@@ -1715,7 +1717,10 @@
void UniqueIDBDatabase::postDatabaseTaskReply(CrossThreadTask&& task)
{
ASSERT(!isMainThread());
- m_databaseReplyQueue.append(WTFMove(task));
+
+ m_databaseReplyQueue.append([protectedThis = makeRef(*this), task = WTFMove(task)]() mutable {
+ task.performTask();
+ });
++m_queuedTaskCount;
m_server.postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::executeNextDatabaseTaskReply));
@@ -1729,14 +1734,12 @@
auto task = m_databaseQueue.tryGetMessage();
ASSERT(task);
- // Performing the task might end up removing the last reference to this.
- Ref<UniqueIDBDatabase> protectedThis(*this);
-
- task->performTask();
+ (*task)();
--m_queuedTaskCount;
- // Release the ref in the main thread to ensure it's deleted there as expected in case of being the last reference.
- callOnMainThread([protectedThis = WTFMove(protectedThis)] {
+ // 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)] {
});
}
@@ -1748,10 +1751,7 @@
auto task = m_databaseReplyQueue.tryGetMessage();
ASSERT(task);
- // Performing the task might end up removing the last reference to this.
- Ref<UniqueIDBDatabase> protectedThis(*this);
-
- task->performTask();
+ (*task)();
--m_queuedTaskCount;
// If this database was force closed (e.g. for a user delete) and there are no more
Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h (218515 => 218516)
--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h 2017-06-19 23:08:53 UTC (rev 218515)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h 2017-06-19 23:19:11 UTC (rev 218516)
@@ -266,8 +266,8 @@
bool m_deleteBackingStoreInProgress { false };
- CrossThreadQueue<CrossThreadTask> m_databaseQueue;
- CrossThreadQueue<CrossThreadTask> m_databaseReplyQueue;
+ CrossThreadQueue<Function<void ()>> m_databaseQueue;
+ CrossThreadQueue<Function<void ()>> m_databaseReplyQueue;
std::atomic<uint64_t> m_queuedTaskCount { 0 };
bool m_hardClosedForUserDelete { false };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
