Race condition in StartWebThread causing crash

https://bugs.webkit.org/show_bug.cgi?id=175852

Reviewed by Mark Lam.

When starting web thread, the main thread waits for completion of web thread initialization
by using pthread_cond_t. However, the main thread may be woken up due to the existence of
the spurious wake up of pthread_cond_t.

Instead, we should use WTF::Lock and WTF::Condition. Since our StartWebThread already calls
WTF::initializeThreading, it is safe to use WTF::Lock and WTF::Condition. And our WTF::Condition
does not have the spurious wake up problem as described in Condition.h.

* platform/ios/wak/WebCoreThread.mm:
(RunWebThread):
(StartWebThread):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index d7de685..e1d851a 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2017-08-23  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Race condition in StartWebThread causing crash
+        https://bugs.webkit.org/show_bug.cgi?id=175852
+
+        Reviewed by Mark Lam.
+
+        When starting web thread, the main thread waits for completion of web thread initialization
+        by using pthread_cond_t. However, the main thread may be woken up due to the existence of
+        the spurious wake up of pthread_cond_t.
+
+        Instead, we should use WTF::Lock and WTF::Condition. Since our StartWebThread already calls
+        WTF::initializeThreading, it is safe to use WTF::Lock and WTF::Condition. And our WTF::Condition
+        does not have the spurious wake up problem as described in Condition.h.
+
+        * platform/ios/wak/WebCoreThread.mm:
+        (RunWebThread):
+        (StartWebThread):
+
 2017-08-23  Brent Fulgham  <bfulgham@apple.com>
 
         Ensure media controls host exists before using it

diff --git a/Source/WebCore/platform/ios/wak/WebCoreThread.mm b/Source/WebCore/platform/ios/wak/WebCoreThread.mm
index c8f05a1..80e4540 100644 (file)
--- a/Source/WebCore/platform/ios/wak/WebCoreThread.mm
+++ b/Source/WebCore/platform/ios/wak/WebCoreThread.mm
@@ -127,8 +127,8 @@ static BOOL sendingDelegateMessage;
 
 static CFRunLoopObserverRef mainRunLoopAutoUnlockObserver;
 
-static pthread_mutex_t startupLock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t startupCondition = PTHREAD_COND_INITIALIZER;
+static StaticLock startupLock;
+static StaticCondition startupCondition;
 
 static WebThreadContext *webThreadContext;
 static pthread_key_t threadContextKey;
@@ -681,14 +681,10 @@ void *RunWebThread(void *arg)
     WebThreadReleaseSource = CFRunLoopSourceCreate(NULL, -1, &ReleaseSourceContext);
     CFRunLoopAddSource(webThreadRunLoop, WebThreadReleaseSource, kCFRunLoopDefaultMode);
 
-    int result = pthread_mutex_lock(&startupLock);
-    ASSERT_WITH_MESSAGE(result == 0, "startup lock failed with code:%d", result);
-
-    result = pthread_cond_signal(&startupCondition);
-    ASSERT_WITH_MESSAGE(result == 0, "startup signal failed with code:%d", result);
-
-    result = pthread_mutex_unlock(&startupLock);
-    ASSERT_WITH_MESSAGE(result == 0, "startup unlock failed with code:%d", result);
+    {
+        LockHolder locker(startupLock);
+        startupCondition.notifyOne();
+    }
 
     while (1)
         CFRunLoopRunInMode(kCFRunLoopDefaultMode, DistantFuture, true);
@@ -758,20 +754,17 @@ static void StartWebThread()
     pthread_attr_setschedparam(&tattr, &param);
 
     // Wait for the web thread to startup completely before we continue.
-    int result = pthread_mutex_lock(&startupLock);
-    ASSERT_WITH_MESSAGE(result == 0, "startup lock failed with code:%d", result);
+    {
+        LockHolder locker(startupLock);
 
-    // Propagate the mainThread's fenv to workers & the web thread.
-    FloatingPointEnvironment::singleton().saveMainThreadEnvironment();
+        // Propagate the mainThread's fenv to workers & the web thread.
+        FloatingPointEnvironment::singleton().saveMainThreadEnvironment();
 
-    pthread_create(&webThread, &tattr, RunWebThread, NULL);
-    pthread_attr_destroy(&tattr);
+        pthread_create(&webThread, &tattr, RunWebThread, NULL);
+        pthread_attr_destroy(&tattr);
 
-    result = pthread_cond_wait(&startupCondition, &startupLock);
-    ASSERT_WITH_MESSAGE(result == 0, "startup wait failed with code:%d", result);
-
-    result = pthread_mutex_unlock(&startupLock);
-    ASSERT_WITH_MESSAGE(result == 0, "startup unlock failed with code:%d", result);
+        startupCondition.wait(startupLock);
+    }
 
     initializeApplicationUIThreadIdentifier();
 }