[WTF] Make isMainThread more reliable
authoryusukesuzuki@slowstart.org <yusukesuzuki@slowstart.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Sep 2018 22:32:34 +0000 (22:32 +0000)
committeryusukesuzuki@slowstart.org <yusukesuzuki@slowstart.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Sep 2018 22:32:34 +0000 (22:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189880

Reviewed by Mark Lam.

.:

* Source/cmake/OptionsCommon.cmake:

Source/WTF:

isMainThread() relied on Thread::current(). This API becomes broken in Windows
when the Thread is about to be destroyed since TLS is already cleared. This causes
a bug since `isMainThread()` is called in Thread::didExit in Windows.

This patch makes this `isMainThread` more reliable in all the platforms. In Windows,
we use `Thread::currentID()` instead of `Thread::current()` since `Thread::currentID`
uses Win32 GetCurrentThreadId directly. In the other system, we use `pthread_main_np`
or `pthread_self` instead.

We also move `holdLock` code inside `if (shouldRemoveThreadFromThreadGroup())`. If
the other thread takes a mutex and destroyed, this `holdLock` waits forever. This problem
only happens in Windows since Windows calls TLS destructor for the main thread.

* WTF.xcodeproj/project.pbxproj:
* wtf/MainThread.cpp:
(WTF::initializeMainThread):
(): Deleted.
(WTF::isMainThread): Deleted.
(WTF::isMainThreadIfInitialized): Deleted.
* wtf/Platform.h:
* wtf/PlatformMac.cmake:
* wtf/Threading.cpp:
(WTF::Thread::didExit):
* wtf/cocoa/MainThreadCocoa.mm: Renamed from Source/WTF/wtf/mac/MainThreadMac.mm.
* wtf/generic/MainThreadGeneric.cpp:
(WTF::initializeMainThreadPlatform):
(WTF::isMainThread):
(WTF::isMainThreadIfInitialized):
* wtf/win/MainThreadWin.cpp:
(WTF::initializeMainThreadPlatform):
(WTF::isMainThread):
(WTF::isMainThreadIfInitialized):

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

ChangeLog
Source/WTF/ChangeLog
Source/WTF/WTF.xcodeproj/project.pbxproj
Source/WTF/wtf/MainThread.cpp
Source/WTF/wtf/Platform.h
Source/WTF/wtf/PlatformMac.cmake
Source/WTF/wtf/Threading.cpp
Source/WTF/wtf/cocoa/MainThreadCocoa.mm [moved from Source/WTF/wtf/mac/MainThreadMac.mm with 99% similarity]
Source/WTF/wtf/generic/MainThreadGeneric.cpp
Source/WTF/wtf/win/MainThreadWin.cpp
Source/cmake/OptionsCommon.cmake

index 8e8693b..e7b9a42 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2018-09-28  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
+
+        [WTF] Make isMainThread more reliable
+        https://bugs.webkit.org/show_bug.cgi?id=189880
+
+        Reviewed by Mark Lam.
+
+        * Source/cmake/OptionsCommon.cmake:
+
 2018-09-21  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         [JSC] Enable LLInt ASM interpreter on X64 and ARM64 in non JIT configuration
index d41d83b..bcb1e2e 100644 (file)
@@ -1,3 +1,43 @@
+2018-09-28  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
+
+        [WTF] Make isMainThread more reliable
+        https://bugs.webkit.org/show_bug.cgi?id=189880
+
+        Reviewed by Mark Lam.
+
+        isMainThread() relied on Thread::current(). This API becomes broken in Windows
+        when the Thread is about to be destroyed since TLS is already cleared. This causes
+        a bug since `isMainThread()` is called in Thread::didExit in Windows.
+
+        This patch makes this `isMainThread` more reliable in all the platforms. In Windows,
+        we use `Thread::currentID()` instead of `Thread::current()` since `Thread::currentID`
+        uses Win32 GetCurrentThreadId directly. In the other system, we use `pthread_main_np`
+        or `pthread_self` instead.
+
+        We also move `holdLock` code inside `if (shouldRemoveThreadFromThreadGroup())`. If
+        the other thread takes a mutex and destroyed, this `holdLock` waits forever. This problem
+        only happens in Windows since Windows calls TLS destructor for the main thread.
+
+        * WTF.xcodeproj/project.pbxproj:
+        * wtf/MainThread.cpp:
+        (WTF::initializeMainThread):
+        (): Deleted.
+        (WTF::isMainThread): Deleted.
+        (WTF::isMainThreadIfInitialized): Deleted.
+        * wtf/Platform.h:
+        * wtf/PlatformMac.cmake:
+        * wtf/Threading.cpp:
+        (WTF::Thread::didExit):
+        * wtf/cocoa/MainThreadCocoa.mm: Renamed from Source/WTF/wtf/mac/MainThreadMac.mm.
+        * wtf/generic/MainThreadGeneric.cpp:
+        (WTF::initializeMainThreadPlatform):
+        (WTF::isMainThread):
+        (WTF::isMainThreadIfInitialized):
+        * wtf/win/MainThreadWin.cpp:
+        (WTF::initializeMainThreadPlatform):
+        (WTF::isMainThread):
+        (WTF::isMainThreadIfInitialized):
+
 2018-09-28  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r236605.
index 9df1a5e..ccf2946 100644 (file)
                A8A473BA151A825B004123FF /* dtoa.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A47297151A825A004123FF /* dtoa.cpp */; };
                A8A473C3151A825B004123FF /* FastMalloc.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A472A1151A825A004123FF /* FastMalloc.cpp */; };
                A8A473D8151A825B004123FF /* HashTable.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A472B8151A825A004123FF /* HashTable.cpp */; };
-               A8A473E4151A825B004123FF /* MainThreadMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = A8A472C5151A825A004123FF /* MainThreadMac.mm */; };
+               A8A473E4151A825B004123FF /* MainThreadCocoa.mm in Sources */ = {isa = PBXBuildFile; fileRef = A8A472C5151A825A004123FF /* MainThreadCocoa.mm */; };
                A8A473E5151A825B004123FF /* MainThread.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A472C6151A825A004123FF /* MainThread.cpp */; };
                A8A473E9151A825B004123FF /* MD5.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A472CA151A825B004123FF /* MD5.cpp */; };
                A8A473EC151A825B004123FF /* MetaAllocator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A472CD151A825B004123FF /* MetaAllocator.cpp */; };
                A8A472BC151A825A004123FF /* InlineASM.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = InlineASM.h; sourceTree = "<group>"; };
                A8A472C1151A825A004123FF /* ListHashSet.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ListHashSet.h; sourceTree = "<group>"; };
                A8A472C3151A825A004123FF /* Locker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Locker.h; sourceTree = "<group>"; };
-               A8A472C5151A825A004123FF /* MainThreadMac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MainThreadMac.mm; sourceTree = "<group>"; };
+               A8A472C5151A825A004123FF /* MainThreadCocoa.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MainThreadCocoa.mm; sourceTree = "<group>"; };
                A8A472C6151A825A004123FF /* MainThread.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MainThread.cpp; sourceTree = "<group>"; };
                A8A472C7151A825B004123FF /* MainThread.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MainThread.h; sourceTree = "<group>"; };
                A8A472C9151A825B004123FF /* MathExtras.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MathExtras.h; sourceTree = "<group>"; };
                                1A428B8B1C8F89DD0051E9EB /* AppKitCompatibilityDeclarations.h */,
                                1ACADD821884480100D8B71D /* DeprecatedSymbolsUsedBySafari.mm */,
                                53534F291EC0E10E00141B2F /* MachExceptions.defs */,
-                               A8A472C5151A825A004123FF /* MainThreadMac.mm */,
                        );
                        path = mac;
                        sourceTree = "<group>";
                                143DDE9720C8BE99007F76FA /* Entitlements.h */,
                                143DDE9520C8BC37007F76FA /* Entitlements.mm */,
                                7A6EBA3320746C34004F9C44 /* MachSendRight.cpp */,
+                               A8A472C5151A825A004123FF /* MainThreadCocoa.mm */,
                                ADF2CE651E39F106006889DB /* MemoryFootprintCocoa.cpp */,
                                AD89B6B91E64150F0090707F /* MemoryPressureHandlerCocoa.mm */,
                                A30D412C1F0DE0BA00B71954 /* SoftLinking.h */,
                                53534F2A1EC0E10E00141B2F /* MachExceptions.defs in Sources */,
                                7A6EBA3420746C34004F9C44 /* MachSendRight.cpp in Sources */,
                                A8A473E5151A825B004123FF /* MainThread.cpp in Sources */,
-                               A8A473E4151A825B004123FF /* MainThreadMac.mm in Sources */,
+                               A8A473E4151A825B004123FF /* MainThreadCocoa.mm in Sources */,
                                A8A473E9151A825B004123FF /* MD5.cpp in Sources */,
                                CD5497AC15857D0300B5BC30 /* MediaTime.cpp in Sources */,
                                ADF2CE671E39F106006889DB /* MemoryFootprintCocoa.cpp in Sources */,
index 5b40cb8..4c54772 100644 (file)
 namespace WTF {
 
 static bool callbacksPaused; // This global variable is only accessed from main thread.
-#if !PLATFORM(COCOA)
-static Thread* mainThread { nullptr };
-#endif
-
 static Lock mainThreadFunctionQueueMutex;
 
 static Deque<Function<void ()>>& functionQueue()
@@ -60,26 +56,11 @@ void initializeMainThread()
 {
     std::call_once(initializeKey, [] {
         initializeThreading();
-#if !PLATFORM(COCOA)
-        mainThread = &Thread::current();
-#endif
         initializeMainThreadPlatform();
         initializeGCThreads();
     });
 }
 
-#if !PLATFORM(COCOA)
-bool isMainThread()
-{
-    return mainThread == &Thread::current();
-}
-
-bool isMainThreadIfInitialized()
-{
-    return isMainThread();
-}
-#endif
-
 #if PLATFORM(COCOA)
 #if !USE(WEB_THREAD)
 void initializeMainThreadToProcessMainThread()
index f13af9d..a0e3c40 100644 (file)
 #define HAVE_TM_GMTOFF 1
 #define HAVE_TM_ZONE 1
 #define HAVE_TIMEGM 1
+#define HAVE_PTHREAD_MAIN_NP 1
 
 #if CPU(X86_64) || CPU(ARM64)
 #define HAVE_INT128_T 1
index c651388..f4d526e 100644 (file)
@@ -45,12 +45,12 @@ list(APPEND WTF_SOURCES
     cocoa/CPUTimeCocoa.cpp
     cocoa/Entitlements.cpp
     cocoa/MachSendRight.cpp
+    cocoa/MainThreadCocoa.mm
     cocoa/MemoryFootprintCocoa.cpp
     cocoa/MemoryPressureHandlerCocoa.mm
     cocoa/WorkQueueCocoa.cpp
 
     mac/DeprecatedSymbolsUsedBySafari.mm
-    mac/MainThreadMac.mm
 
     text/cf/AtomicStringImplCF.cpp
     text/cf/StringCF.cpp
index 078f73a..409ba75 100644 (file)
@@ -185,28 +185,30 @@ static bool shouldRemoveThreadFromThreadGroup()
 void Thread::didExit()
 {
     if (shouldRemoveThreadFromThreadGroup()) {
-        Vector<std::shared_ptr<ThreadGroup>> threadGroups;
         {
-            auto locker = holdLock(m_mutex);
-            for (auto& threadGroup : m_threadGroups) {
-                // If ThreadGroup is just being destroyed,
-                // we do not need to perform unregistering.
-                if (auto retained = threadGroup.lock())
-                    threadGroups.append(WTFMove(retained));
+            Vector<std::shared_ptr<ThreadGroup>> threadGroups;
+            {
+                auto locker = holdLock(m_mutex);
+                for (auto& threadGroup : m_threadGroups) {
+                    // If ThreadGroup is just being destroyed,
+                    // we do not need to perform unregistering.
+                    if (auto retained = threadGroup.lock())
+                        threadGroups.append(WTFMove(retained));
+                }
+                m_isShuttingDown = true;
+            }
+            for (auto& threadGroup : threadGroups) {
+                auto threadGroupLocker = holdLock(threadGroup->getLock());
+                auto locker = holdLock(m_mutex);
+                threadGroup->m_threads.remove(*this);
             }
-            m_isShuttingDown = true;
-        }
-        for (auto& threadGroup : threadGroups) {
-            auto threadGroupLocker = holdLock(threadGroup->getLock());
-            auto locker = holdLock(m_mutex);
-            threadGroup->m_threads.remove(*this);
         }
-    }
 
-    // We would like to say "thread is exited" after unregistering threads from thread groups.
-    // So we need to separate m_isShuttingDown from m_didExit.
-    auto locker = holdLock(m_mutex);
-    m_didExit = true;
+        // We would like to say "thread is exited" after unregistering threads from thread groups.
+        // So we need to separate m_isShuttingDown from m_didExit.
+        auto locker = holdLock(m_mutex);
+        m_didExit = true;
+    }
 }
 
 ThreadGroupAddResult Thread::addToThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup& threadGroup)
similarity index 99%
rename from Source/WTF/wtf/mac/MainThreadMac.mm
rename to Source/WTF/wtf/cocoa/MainThreadCocoa.mm
index cdb42af..0c9263f 100644 (file)
@@ -25,7 +25,7 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+
 #import "config.h"
 #import "MainThread.h"
 
index 91ad2e5..6d8e40f 100644 (file)
  */
 
 #include "config.h"
-#include "MainThread.h"
+#include <pthread.h>
+#if HAVE(PTHREAD_NP_H)
+#include <pthread_np.h>
+#endif
 
 #include <wtf/RunLoop.h>
 #if USE(GLIB)
 
 namespace WTF {
 
+#if !HAVE(PTHREAD_MAIN_NP)
+static pthread_t mainThread;
+#endif
+
 class MainThreadDispatcher {
 public:
     MainThreadDispatcher()
@@ -65,6 +72,23 @@ private:
 
 void initializeMainThreadPlatform()
 {
+#if !HAVE(PTHREAD_MAIN_NP)
+    mainThread = pthread_self();
+#endif
+}
+
+bool isMainThread()
+{
+#if HAVE(PTHREAD_MAIN_NP)
+    return pthread_main_np();
+#else
+    return pthread_equal(pthread_self(), mainThread);
+#endif
+}
+
+bool isMainThreadIfInitialized()
+{
+    return isMainThread();
 }
 
 void scheduleDispatchFunctionsOnMainThread()
index a6473d0..e0df322 100644 (file)
@@ -39,6 +39,7 @@ namespace WTF {
 static HWND threadingWindowHandle;
 static UINT threadingFiredMessage;
 const LPCWSTR kThreadingWindowClassName = L"ThreadingWindowClass";
+static ThreadIdentifier mainThread { 0 };
 
 LRESULT CALLBACK ThreadingWindowWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
 {
@@ -64,9 +65,21 @@ void initializeMainThreadPlatform()
         CW_USEDEFAULT, 0, CW_USEDEFAULT, 0, HWND_MESSAGE, 0, 0, 0);
     threadingFiredMessage = RegisterWindowMessageW(L"com.apple.WebKit.MainThreadFired");
 
+    mainThread = Thread::currentID();
+
     Thread::initializeCurrentThreadInternal("Main Thread");
 }
 
+bool isMainThread()
+{
+    return mainThread == Thread::currentID();
+}
+
+bool isMainThreadIfInitialized()
+{
+    return isMainThread();
+}
+
 void scheduleDispatchFunctionsOnMainThread()
 {
     ASSERT(threadingWindowHandle);
index 76303c3..d03d21b 100644 (file)
@@ -134,6 +134,7 @@ WEBKIT_CHECK_HAVE_FUNCTION(HAVE_VASPRINTF vasprintf)
 
 # Check for symbols
 WEBKIT_CHECK_HAVE_SYMBOL(HAVE_REGEX_H regexec regex.h)
+WEBKIT_CHECK_HAVE_SYMBOL(HAVE_PTHREAD_MAIN_NP pthread_main_np pthread_np.h)
 # Windows has signal.h but is missing symbols that are used in calls to signal.
 WEBKIT_CHECK_HAVE_SYMBOL(HAVE_SIGNAL_H SIGTRAP signal.h)