Title: [141169] branches/chromium/1364/Source
- Revision
- 141169
- Author
- jsb...@chromium.org
- Date
- 2013-01-29 15:00:10 -0800 (Tue, 29 Jan 2013)
Log Message
Merge 140483
> Prevent race condition during Worker shutdown
> https://bugs.webkit.org/show_bug.cgi?id=107577
>
> Reviewed by Dmitry Titov.
>
> Source/WebCore:
>
> During worker shutdown, from the main thread a cleanup task is posted followed by
> terminating the message queue, which prevents further tasks from being processed. It was
> possible for another task be posted by another thread between the main thread calls
> to postTask and terminate(), which would cause that task to run after cleanup. Expose
> a new WTF::MessageQueue::appendAndKill() method which keeps a mutex around the two steps,
> and use that during worker shutdown.
>
> No reliable tests for the race - problem identified by inspection of user crash stacks.
>
> * workers/WorkerRunLoop.cpp:
> (WebCore::WorkerRunLoop::postTaskAndTerminate): New method, uses MessageQueue::appendAndKill()
> * workers/WorkerRunLoop.h:
> * workers/WorkerThread.cpp:
> (WebCore::WorkerThread::stop): Uses postTaskAndTerminate() to avoid race.
>
> Source/WTF:
>
> Add MessageQueue::appendAndKill() which wraps those two steps with a mutex so other
> threads can't sneak a message in between.
>
> * wtf/MessageQueue.h: Added appendAndKill() method.
>
TBR=jsb...@chromium.org
Review URL: https://codereview.chromium.org/12096050
Modified Paths
Diff
Modified: branches/chromium/1364/Source/WTF/wtf/MessageQueue.h (141168 => 141169)
--- branches/chromium/1364/Source/WTF/wtf/MessageQueue.h 2013-01-29 22:52:58 UTC (rev 141168)
+++ branches/chromium/1364/Source/WTF/wtf/MessageQueue.h 2013-01-29 23:00:10 UTC (rev 141169)
@@ -55,6 +55,7 @@
~MessageQueue();
void append(PassOwnPtr<DataType>);
+ void appendAndKill(PassOwnPtr<DataType>);
bool appendAndCheckEmpty(PassOwnPtr<DataType>);
void prepend(PassOwnPtr<DataType>);
@@ -98,6 +99,15 @@
m_condition.signal();
}
+ template<typename DataType>
+ inline void MessageQueue<DataType>::appendAndKill(PassOwnPtr<DataType> message)
+ {
+ MutexLocker lock(m_mutex);
+ m_queue.append(message.leakPtr());
+ m_killed = true;
+ m_condition.broadcast();
+ }
+
// Returns true if the queue was empty before the item was added.
template<typename DataType>
inline bool MessageQueue<DataType>::appendAndCheckEmpty(PassOwnPtr<DataType> message)
Modified: branches/chromium/1364/Source/WebCore/workers/WorkerRunLoop.cpp (141168 => 141169)
--- branches/chromium/1364/Source/WebCore/workers/WorkerRunLoop.cpp 2013-01-29 22:52:58 UTC (rev 141168)
+++ branches/chromium/1364/Source/WebCore/workers/WorkerRunLoop.cpp 2013-01-29 23:00:10 UTC (rev 141169)
@@ -201,6 +201,11 @@
postTaskForMode(task, defaultMode());
}
+void WorkerRunLoop::postTaskAndTerminate(PassOwnPtr<ScriptExecutionContext::Task> task)
+{
+ m_messageQueue.appendAndKill(Task::create(task, defaultMode().isolatedCopy()));
+}
+
void WorkerRunLoop::postTaskForMode(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
{
m_messageQueue.append(Task::create(task, mode.isolatedCopy()));
Modified: branches/chromium/1364/Source/WebCore/workers/WorkerRunLoop.h (141168 => 141169)
--- branches/chromium/1364/Source/WebCore/workers/WorkerRunLoop.h 2013-01-29 22:52:58 UTC (rev 141168)
+++ branches/chromium/1364/Source/WebCore/workers/WorkerRunLoop.h 2013-01-29 23:00:10 UTC (rev 141169)
@@ -61,6 +61,7 @@
bool terminated() const { return m_messageQueue.killed(); }
void postTask(PassOwnPtr<ScriptExecutionContext::Task>);
+ void postTaskAndTerminate(PassOwnPtr<ScriptExecutionContext::Task>);
void postTaskForMode(PassOwnPtr<ScriptExecutionContext::Task>, const String& mode);
unsigned long createUniqueId() { return ++m_uniqueId; }
Modified: branches/chromium/1364/Source/WebCore/workers/WorkerThread.cpp (141168 => 141169)
--- branches/chromium/1364/Source/WebCore/workers/WorkerThread.cpp 2013-01-29 22:52:58 UTC (rev 141168)
+++ branches/chromium/1364/Source/WebCore/workers/WorkerThread.cpp 2013-01-29 23:00:10 UTC (rev 141169)
@@ -267,7 +267,8 @@
#if ENABLE(SQL_DATABASE)
DatabaseManager::manager().interruptAllDatabasesForContext(m_workerContext.get());
#endif
- m_runLoop.postTask(WorkerThreadShutdownStartTask::create());
+ m_runLoop.postTaskAndTerminate(WorkerThreadShutdownStartTask::create());
+ return;
}
m_runLoop.terminate();
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
