Skip to content

Commit

Permalink
WorkQueueGeneric's platformInvalidate() can deadlock when called on t…
Browse files Browse the repository at this point in the history
…he 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):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@210271 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
utatane.tea@gmail.com authored and zdobersek committed Jan 5, 2017
1 parent 2d9d5d0 commit 9d4d2cd
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 15 deletions.
2 changes: 0 additions & 2 deletions Source/WTF/wtf/WorkQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,6 @@ class WorkQueue final : public FunctionDispatcher {
Lock m_initializeRunLoopConditionMutex;
Condition m_initializeRunLoopCondition;
RunLoop* m_runLoop;
Lock m_terminateRunLoopConditionMutex;
Condition m_terminateRunLoopCondition;
#endif
};

Expand Down
15 changes: 2 additions & 13 deletions Source/WTF/wtf/generic/WorkQueueGeneric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
33 changes: 33 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 9d4d2cd

Please sign in to comment.