Crash in worker tests handling the m_stoppedCallback.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Nov 2017 00:28:30 +0000 (00:28 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Nov 2017 00:28:30 +0000 (00:28 +0000)
<rdar://problem/35590875> and https://bugs.webkit.org/show_bug.cgi?id=179798

Reviewed by Chris Dumez.

No new tests (Covered by existing tests).

Protect manipulation of m_stoppedCallback with m_threadCreationAndWorkerGlobalScopeMutex.

* workers/WorkerThread.cpp:
(WebCore::WorkerThread::workerThread):
(WebCore::WorkerThread::stop):

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

Source/WebCore/ChangeLog
Source/WebCore/workers/WorkerThread.cpp

index 08851f8..098e47e 100644 (file)
@@ -1,3 +1,18 @@
+2017-11-16  Brady Eidson  <beidson@apple.com>
+
+        Crash in worker tests handling the m_stoppedCallback.
+        <rdar://problem/35590875> and https://bugs.webkit.org/show_bug.cgi?id=179798
+
+        Reviewed by Chris Dumez.
+
+        No new tests (Covered by existing tests).
+
+        Protect manipulation of m_stoppedCallback with m_threadCreationAndWorkerGlobalScopeMutex.
+
+        * workers/WorkerThread.cpp:
+        (WebCore::WorkerThread::workerThread):
+        (WebCore::WorkerThread::stop):
+
 2017-11-16  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Express time in milliseconds through the API
index 7211bad..5e0e462 100644 (file)
@@ -219,6 +219,9 @@ void WorkerThread::workerThread()
         // context will trigger the main thread to race against us to delete the WorkerThread
         // object, and the WorkerThread object owns the mutex we need to unlock after this.
         workerGlobalScopeToDelete = WTFMove(m_workerGlobalScope);
+
+        if (m_stoppedCallback)
+            callOnMainThread(WTFMove(m_stoppedCallback));
     }
 
     // The below assignment will destroy the context, which will in turn notify messaging proxy.
@@ -228,9 +231,6 @@ void WorkerThread::workerThread()
     // Clean up WebCore::ThreadGlobalData before WTF::Thread goes away!
     threadGlobalData().destroy();
 
-    if (m_stoppedCallback)
-        callOnMainThread(WTFMove(m_stoppedCallback));
-
     // The thread object may be already destroyed from notification now, don't try to access "this".
     protector->detach();
 }
@@ -259,14 +259,14 @@ void WorkerThread::runEventLoop()
 
 void WorkerThread::stop(WTF::Function<void()>&& stoppedCallback)
 {
-    ASSERT(!m_stoppedCallback);
-    m_stoppedCallback = WTFMove(stoppedCallback);
-
     // Mutex protection is necessary to ensure that m_workerGlobalScope isn't changed by
     // WorkerThread::workerThread() while we're accessing it. Note also that stop() can
     // be called before m_workerGlobalScope is fully created.
     LockHolder lock(m_threadCreationAndWorkerGlobalScopeMutex);
 
+    ASSERT(!m_stoppedCallback);
+    m_stoppedCallback = WTFMove(stoppedCallback);
+
     // Ensure that tasks are being handled by thread event loop. If script execution weren't forbidden, a while(1) loop in JS could keep the thread alive forever.
     if (m_workerGlobalScope) {
         m_workerGlobalScope->script()->scheduleExecutionTermination();