Title: [140483] trunk/Source
- Revision
- 140483
- Author
- jsb...@chromium.org
- Date
- 2013-01-22 15:45:45 -0800 (Tue, 22 Jan 2013)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (140482 => 140483)
--- trunk/Source/WTF/ChangeLog 2013-01-22 23:44:27 UTC (rev 140482)
+++ trunk/Source/WTF/ChangeLog 2013-01-22 23:45:45 UTC (rev 140483)
@@ -1,3 +1,15 @@
+2013-01-22 Joshua Bell <jsb...@chromium.org>
+
+ Prevent race condition during Worker shutdown
+ https://bugs.webkit.org/show_bug.cgi?id=107577
+
+ Reviewed by Dmitry Titov.
+
+ 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.
+
2013-01-22 Roger Fong <roger_f...@apple.com>
WTF project files and property sheets for getting WebKit to compile in VS2010.
Modified: trunk/Source/WTF/wtf/MessageQueue.h (140482 => 140483)
--- trunk/Source/WTF/wtf/MessageQueue.h 2013-01-22 23:44:27 UTC (rev 140482)
+++ trunk/Source/WTF/wtf/MessageQueue.h 2013-01-22 23:45:45 UTC (rev 140483)
@@ -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: trunk/Source/WebCore/ChangeLog (140482 => 140483)
--- trunk/Source/WebCore/ChangeLog 2013-01-22 23:44:27 UTC (rev 140482)
+++ trunk/Source/WebCore/ChangeLog 2013-01-22 23:45:45 UTC (rev 140483)
@@ -1,3 +1,25 @@
+2013-01-22 Joshua Bell <jsb...@chromium.org>
+
+ Prevent race condition during Worker shutdown
+ https://bugs.webkit.org/show_bug.cgi?id=107577
+
+ Reviewed by Dmitry Titov.
+
+ 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.
+
2013-01-22 Tony Chang <t...@chromium.org>
Unreviewed, rolling out r140171.
Modified: trunk/Source/WebCore/workers/WorkerRunLoop.cpp (140482 => 140483)
--- trunk/Source/WebCore/workers/WorkerRunLoop.cpp 2013-01-22 23:44:27 UTC (rev 140482)
+++ trunk/Source/WebCore/workers/WorkerRunLoop.cpp 2013-01-22 23:45:45 UTC (rev 140483)
@@ -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: trunk/Source/WebCore/workers/WorkerRunLoop.h (140482 => 140483)
--- trunk/Source/WebCore/workers/WorkerRunLoop.h 2013-01-22 23:44:27 UTC (rev 140482)
+++ trunk/Source/WebCore/workers/WorkerRunLoop.h 2013-01-22 23:45:45 UTC (rev 140483)
@@ -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: trunk/Source/WebCore/workers/WorkerThread.cpp (140482 => 140483)
--- trunk/Source/WebCore/workers/WorkerThread.cpp 2013-01-22 23:44:27 UTC (rev 140482)
+++ trunk/Source/WebCore/workers/WorkerThread.cpp 2013-01-22 23:45:45 UTC (rev 140483)
@@ -271,7 +271,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
http://lists.webkit.org/mailman/listinfo/webkit-changes
