Changeset 221083 in webkit

Timestamp:

Aug 23, 2017 10:41:39 AM (7 years ago)

Author:

Yusuke Suzuki

Message:

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/ios/wak/WebCoreThread.mm (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/platform/ios/wak/WebCoreThread.mm

@@ -128 +128 @@
 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;
@@ -682 +682 @@
     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)
@@ -759 +755 @@
 
     // 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);
-
-    // Propagate the mainThread's fenv to workers & the web thread.
-    FloatingPointEnvironment::singleton().saveMainThreadEnvironment();
-
-    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);
+    {
+        LockHolder locker(startupLock);
+
+        // Propagate the mainThread's fenv to workers & the web thread.
+        FloatingPointEnvironment::singleton().saveMainThreadEnvironment();
+
+        pthread_create(&webThread, &tattr, RunWebThread, NULL);
+        pthread_attr_destroy(&tattr);
+
+        startupCondition.wait(startupLock);
+    }
 
     initializeApplicationUIThreadIdentifier();