WorkQueueGeneric's platformInvalidate() can deadlock when called on the RunLoop's...
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jan 2017 07:52:05 +0000 (07:52 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jan 2017 07:52:05 +0000 (07:52 +0000)
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):

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

Source/WTF/ChangeLog
Source/WTF/wtf/WorkQueue.h
Source/WTF/wtf/generic/WorkQueueGeneric.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp

index 65b6b71..3885848 100644 (file)
@@ -1,3 +1,28 @@
+2017-01-03  Yusuke Suzuki  <utatane.tea@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  <sam@webkit.org>
 
         Make WTF::Expected support Ref template parameters
index 5ac32f8..d9184b4 100644 (file)
@@ -116,8 +116,6 @@ private:
     Lock m_initializeRunLoopConditionMutex;
     Condition m_initializeRunLoopCondition;
     RunLoop* m_runLoop;
-    Lock m_terminateRunLoopConditionMutex;
-    Condition m_terminateRunLoopCondition;
 #endif
 };
 
index 5c7b9b7..4eeb302 100644 (file)
@@ -56,25 +56,14 @@ void WorkQueue::platformInitialize(const char* name, Type, QOS)
             m_initializeRunLoopCondition.notifyOne();
         }
         m_runLoop->run();
-        {
-            LockHolder locker(m_terminateRunLoopConditionMutex);
-            m_runLoop = nullptr;
-            m_terminateRunLoopCondition.notifyOne();
-        }
     });
     m_initializeRunLoopCondition.wait(m_initializeRunLoopConditionMutex);
 }
 
 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;
index ea06bd2..e6e9442 100644 (file)
@@ -1,3 +1,13 @@
+2017-01-03  Yusuke Suzuki  <utatane.tea@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  <aestes@apple.com>
 
         Place all the Cocoa WebCore API tests in the same directory
index 74f5dc2..7e277e9 100644 (file)
@@ -200,4 +200,37 @@ TEST(WTF_WorkQueue, DispatchAfter)
     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