Title: [210271] trunk
- Revision
- 210271
- Author
- utatane....@gmail.com
- Date
- 2017-01-03 23:52:05 -0800 (Tue, 03 Jan 2017)
Log Message
WorkQueueGeneric's platformInvalidate() can deadlock when called on the RunLoop's thread
https://bugs.webkit.org/show_bug.cgi?id=166645
Reviewed by Carlos Garcia Campos.
Source/WTF:
WorkQueue can be destroyed on its invoking thread itself.
The scenario is the following.
1. Create WorkQueue (in thread A).
2. Dispatch a task (in thread A, dispatching a task to thread B).
3. Deref in thread A.
4. The task is executed in thread B.
5. Deref in thread B.
6. The WorkQueue is destroyed, calling platformInvalidate in thread B.
In that case, if platformInvalidate waits thread B's termination, it causes deadlock.
We do not need to wait the thread termination.
* wtf/WorkQueue.h:
* wtf/generic/WorkQueueGeneric.cpp:
(WorkQueue::platformInitialize):
(WorkQueue::platformInvalidate):
Tools:
* TestWebKitAPI/Tests/WTF/WorkQueue.cpp:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (210270 => 210271)
--- trunk/Source/WTF/ChangeLog 2017-01-04 06:41:01 UTC (rev 210270)
+++ trunk/Source/WTF/ChangeLog 2017-01-04 07:52:05 UTC (rev 210271)
@@ -1,3 +1,28 @@
+2017-01-03 Yusuke Suzuki <utatane....@gmail.com>
+
+ WorkQueueGeneric's platformInvalidate() can deadlock when called on the RunLoop's thread
+ https://bugs.webkit.org/show_bug.cgi?id=166645
+
+ Reviewed by Carlos Garcia Campos.
+
+ WorkQueue can be destroyed on its invoking thread itself.
+ The scenario is the following.
+
+ 1. Create WorkQueue (in thread A).
+ 2. Dispatch a task (in thread A, dispatching a task to thread B).
+ 3. Deref in thread A.
+ 4. The task is executed in thread B.
+ 5. Deref in thread B.
+ 6. The WorkQueue is destroyed, calling platformInvalidate in thread B.
+
+ In that case, if platformInvalidate waits thread B's termination, it causes deadlock.
+ We do not need to wait the thread termination.
+
+ * wtf/WorkQueue.h:
+ * wtf/generic/WorkQueueGeneric.cpp:
+ (WorkQueue::platformInitialize):
+ (WorkQueue::platformInvalidate):
+
2017-01-03 Sam Weinig <s...@webkit.org>
Make WTF::Expected support Ref template parameters
Modified: trunk/Source/WTF/wtf/WorkQueue.h (210270 => 210271)
--- trunk/Source/WTF/wtf/WorkQueue.h 2017-01-04 06:41:01 UTC (rev 210270)
+++ trunk/Source/WTF/wtf/WorkQueue.h 2017-01-04 07:52:05 UTC (rev 210271)
@@ -116,8 +116,6 @@
Lock m_initializeRunLoopConditionMutex;
Condition m_initializeRunLoopCondition;
RunLoop* m_runLoop;
- Lock m_terminateRunLoopConditionMutex;
- Condition m_terminateRunLoopCondition;
#endif
};
Modified: trunk/Source/WTF/wtf/generic/WorkQueueGeneric.cpp (210270 => 210271)
--- trunk/Source/WTF/wtf/generic/WorkQueueGeneric.cpp 2017-01-04 06:41:01 UTC (rev 210270)
+++ trunk/Source/WTF/wtf/generic/WorkQueueGeneric.cpp 2017-01-04 07:52:05 UTC (rev 210271)
@@ -56,11 +56,6 @@
m_initializeRunLoopCondition.notifyOne();
}
m_runLoop->run();
- {
- LockHolder locker(m_terminateRunLoopConditionMutex);
- m_runLoop = nullptr;
- m_terminateRunLoopCondition.notifyOne();
- }
});
m_initializeRunLoopCondition.wait(m_initializeRunLoopConditionMutex);
}
@@ -67,14 +62,8 @@
void WorkQueue::platformInvalidate()
{
- {
- LockHolder locker(m_terminateRunLoopConditionMutex);
- if (m_runLoop) {
- m_runLoop->stop();
- m_terminateRunLoopCondition.wait(m_terminateRunLoopConditionMutex);
- }
- }
-
+ if (m_runLoop)
+ m_runLoop->stop();
if (m_workQueueThread) {
detachThread(m_workQueueThread);
m_workQueueThread = 0;
Modified: trunk/Tools/ChangeLog (210270 => 210271)
--- trunk/Tools/ChangeLog 2017-01-04 06:41:01 UTC (rev 210270)
+++ trunk/Tools/ChangeLog 2017-01-04 07:52:05 UTC (rev 210271)
@@ -1,3 +1,13 @@
+2017-01-03 Yusuke Suzuki <utatane....@gmail.com>
+
+ WorkQueueGeneric's platformInvalidate() can deadlock when called on the RunLoop's thread
+ https://bugs.webkit.org/show_bug.cgi?id=166645
+
+ Reviewed by Carlos Garcia Campos.
+
+ * TestWebKitAPI/Tests/WTF/WorkQueue.cpp:
+ (TestWebKitAPI::TEST):
+
2017-01-03 Andy Estes <aes...@apple.com>
Place all the Cocoa WebCore API tests in the same directory
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp (210270 => 210271)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp 2017-01-04 06:41:01 UTC (rev 210270)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp 2017-01-04 07:52:05 UTC (rev 210271)
@@ -200,4 +200,37 @@
EXPECT_STREQ(dispatchAfterLabel, m_functionCallOrder[1].c_str());
}
+TEST(WTF_WorkQueue, DestroyOnSelf)
+{
+ Lock lock;
+ Condition dispatchAfterTestStarted;
+ Condition dispatchAfterTestCompleted;
+ bool started = false;
+ bool completed = false;
+
+ {
+ LockHolder locker(lock);
+ {
+ auto queue = WorkQueue::create("com.apple.WebKit.Test.dispatchAfter");
+ queue->dispatchAfter(std::chrono::milliseconds(500), [&](void) {
+ LockHolder locker(lock);
+ dispatchAfterTestStarted.wait(lock, [&] {
+ return started;
+ });
+ completed = true;
+ dispatchAfterTestCompleted.notifyOne();
+ });
+ }
+ started = true;
+ dispatchAfterTestStarted.notifyOne();
+ }
+ {
+ LockHolder locker(lock);
+ dispatchAfterTestCompleted.wait(lock, [&] {
+ return completed;
+ });
+ WTF::sleep(0.1);
+ }
+}
+
} // namespace TestWebKitAPI
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
