Put ThreadGlobalData in Thread

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

Reviewed by Darin Adler and Mark Lam.

This patch puts ThreadGlobalData into Thread's m_clientData field.
Thread is stored in fast TLS, so accessing to this field is quite fast
compared to the current ThreadSpecific one.

At the same time, this change can remove a hack in ThreadGlobalData.
Previously worker thread needs to tear down ThreadGlobalData explicitly
because Thread::current() can be destroyed earlier than ThreadGlobalData.
In that case, EventNames etc., which accesses to AtomString's destruction
can have problem because of lack of Thread::current() access. But since
we now move it to Thread, we can control how it is destroyed with Thread::current()
precisely, and we can explicitly destroy it before Thread::current() is fully
cleared. So we do not need to call it explicitly anymore.
Currently, we are calling it just to make ThreadGlobalData destroyed for debugging.

* Source/WebCore/PAL/pal/ThreadGlobalData.cpp:
(PAL::ThreadGlobalData::ThreadGlobalData):
(PAL::ThreadGlobalData::destroy): Deleted.
* Source/WebCore/PAL/pal/ThreadGlobalData.h:
(PAL::ThreadGlobalData::ThreadGlobalData::cachedConverterICU): Deleted.
* Source/WTF/wtf/Threading.h:
(WTF::Thread::Thread):
* Source/WTF/wtf/posix/ThreadingPOSIX.cpp:
(WTF::Thread::destructTLS):
* Source/WTF/wtf/win/ThreadingWin.cpp:
(WTF::Thread::ThreadHolder::~ThreadHolder):
* Source/WebCore/platform/ThreadGlobalData.cpp:
(WebCore::ThreadGlobalData::destroy):
(WebCore::ThreadGlobalData::setWebCoreThreadData):
(WebCore::threadGlobalData):

Canonical link: https://commits.webkit.org/250571@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294213 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog

@@ -1,3 +1,17 @@
+2022-05-08  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        Put ThreadGlobalData in Thread
+        https://bugs.webkit.org/show_bug.cgi?id=240116
+
+        Reviewed by Darin Adler and Mark Lam.
+
+        * wtf/Threading.h:
+        (WTF::Thread::Thread):
+        * wtf/posix/ThreadingPOSIX.cpp:
+        (WTF::Thread::destructTLS):
+        * wtf/win/ThreadingWin.cpp:
+        (WTF::Thread::ThreadHolder::~ThreadHolder):
+
 2022-05-14  Tyler Wilcock  <tyler_w@apple.com>
 
         AX: Remove CSSDisplayContentsAXSupportEnabled flag

Source/WTF/wtf/Threading.h

@@ -109,6 +109,11 @@ class WTF_CAPABILITY("is current") Thread : public ThreadSafeRefCounted<Thread>
     friend class ThreadGroup;
     friend WTF_EXPORT_PRIVATE void initialize();
 
+    class ClientData : public ThreadSafeRefCounted<ClientData> {
+    public:
+        virtual ~ClientData() = default;
+    };
+
     WTF_EXPORT_PRIVATE ~Thread();
 
     enum class QOS {
@@ -347,13 +352,12 @@ class WTF_CAPABILITY("is current") Thread : public ThreadSafeRefCounted<Thread>
     static Lock s_allThreadsLock;
 
     JoinableState m_joinableState { Joinable };
-    bool m_isShuttingDown : 1;
-    bool m_didExit : 1;
-    bool m_isDestroyedOnce : 1;
-    bool m_isCompilationThread: 1;
-    unsigned m_gcThreadType : 2;
-
-    bool m_didUnregisterFromAllThreads { false };
+    bool m_isShuttingDown : 1 { false };
+    bool m_didExit : 1 { false };
+    bool m_isDestroyedOnce : 1 { false };
+    bool m_isCompilationThread: 1 { false };
+    bool m_didUnregisterFromAllThreads : 1 { false };
+    unsigned m_gcThreadType : 2 { static_cast<unsigned>(GCThreadType::None) };
 
     // Lock & ParkingLot rely on ThreadSpecific. But Thread object can be destroyed even after ThreadSpecific things are destroyed.
     // Use WordLock since WordLock does not depend on ThreadSpecific and this "Thread".
@@ -388,15 +392,11 @@ class WTF_CAPABILITY("is current") Thread : public ThreadSafeRefCounted<Thread>
     void* m_savedLastStackTop;
 public:
     void* m_apiData { nullptr };
+    RefPtr<ClientData> m_clientData { nullptr };
 };
 
 inline Thread::Thread()
-    : m_isShuttingDown(false)
-    , m_didExit(false)
-    , m_isDestroyedOnce(false)
-    , m_isCompilationThread(false)
-    , m_gcThreadType(static_cast<unsigned>(GCThreadType::None))
-    , m_uid(++s_uid)
+    : m_uid(++s_uid)
 {
 }
 

Source/WTF/wtf/posix/ThreadingPOSIX.cpp

@@ -556,6 +556,10 @@ void Thread::destructTLS(void* data)
     _pthread_setspecific_direct(WTF_THREAD_DATA_KEY, thread);
     pthread_key_init_np(WTF_THREAD_DATA_KEY, &destructTLS);
 #endif
+    // Destructor of ClientData can rely on Thread::current() (e.g. AtomStringTable).
+    // We destroy it after re-setting Thread::current() so that we can ensure destruction
+    // can still access to it.
+    thread->m_clientData = nullptr;
 }
 
 Mutex::~Mutex()

Source/WTF/wtf/win/ThreadingWin.cpp

@@ -269,6 +269,7 @@ struct Thread::ThreadHolder {
         // deadlock.
         if (isMainThread())
             return;
+        thread->m_clientData = nullptr;
         if (thread) {
             thread->specificStorage().destroySlots();
             thread->didExit();

Source/WebCore/ChangeLog

@@ -1,3 +1,29 @@
+2022-05-08  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        Put ThreadGlobalData in Thread
+        https://bugs.webkit.org/show_bug.cgi?id=240116
+
+        Reviewed by Darin Adler and Mark Lam.
+
+        This patch puts ThreadGlobalData into Thread's m_clientData field.
+        Thread is stored in fast TLS, so accessing to this field is quite fast
+        compared to the current ThreadSpecific one.
+
+        At the same time, this change can remove a hack in ThreadGlobalData.
+        Previously worker thread needs to tear down ThreadGlobalData explicitly
+        because Thread::current() can be destroyed earlier than ThreadGlobalData.
+        In that case, EventNames etc., which accesses to AtomString's destruction
+        can have problem because of lack of Thread::current() access. But since
+        we now move it to Thread, we can control how it is destroyed with Thread::current()
+        precisely, and we can explicitly destroy it before Thread::current() is fully
+        cleared. So we do not need to call it explicitly anymore.
+        Currently, we are calling it just to make ThreadGlobalData destroyed for debugging.
+
+        * platform/ThreadGlobalData.cpp:
+        (WebCore::ThreadGlobalData::destroy):
+        (WebCore::ThreadGlobalData::setWebCoreThreadData):
+        (WebCore::threadGlobalData):
+
 2022-05-14  Alan Bujtas  <zalan@apple.com>
 
         Changing text color and removing line-clamp on hover causes text to disappear permanently

Source/WebCore/PAL/ChangeLog

@@ -1,3 +1,16 @@
+2022-05-08  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        Put ThreadGlobalData in Thread
+        https://bugs.webkit.org/show_bug.cgi?id=240116
+
+        Reviewed by Darin Adler and Mark Lam.
+
+        * pal/ThreadGlobalData.cpp:
+        (PAL::ThreadGlobalData::ThreadGlobalData):
+        (PAL::ThreadGlobalData::destroy): Deleted.
+        * pal/ThreadGlobalData.h:
+        (PAL::ThreadGlobalData::ThreadGlobalData::cachedConverterICU): Deleted.
+
 2022-05-12  Eric Carlson  <eric.carlson@apple.com>
 
         [macOS] Remove support for deprecated ScreenCaptureKit API

Source/WebCore/PAL/pal/ThreadGlobalData.cpp

@@ -38,18 +38,8 @@ namespace PAL {
 ThreadGlobalData::ThreadGlobalData()
     : m_cachedConverterICU(makeUnique<ICUConverterWrapper>())
 {
-    // This constructor will have been called on the main thread before being called on
-    // any other thread, and is only called once per thread - this makes this a convenient
-    // point to call methods that internally perform a one-time initialization that is not
-    // threadsafe.
-    Thread::current();
 }
 
 ThreadGlobalData::~ThreadGlobalData() = default;
 
-void ThreadGlobalData::destroy()
-{
-    m_cachedConverterICU = nullptr;
-}
-
 } // namespace PAL

Source/WebCore/PAL/pal/ThreadGlobalData.h

@@ -27,17 +27,14 @@
 #pragma once
 
 #include <wtf/ThreadSafeRefCounted.h>
+#include <wtf/Threading.h>
 #include <wtf/text/StringHash.h>
 
 namespace PAL {
 
 struct ICUConverterWrapper;
 
-#if USE(WEB_THREAD)
-class ThreadGlobalData : public ThreadSafeRefCounted<ThreadGlobalData> {
-#else
-class ThreadGlobalData {
-#endif
+class ThreadGlobalData : public WTF::Thread::ClientData {
     WTF_MAKE_NONCOPYABLE(ThreadGlobalData);
     WTF_MAKE_FAST_ALLOCATED;
 public:
@@ -47,7 +44,6 @@ class ThreadGlobalData {
 
 protected:
     PAL_EXPORT ThreadGlobalData();
-    void destroy(); // called on workers to clean up the ThreadGlobalData before the thread exits.
 
 private:
     PAL_EXPORT friend ThreadGlobalData& threadGlobalData();

Source/WebCore/platform/ThreadGlobalData.cpp

@@ -34,7 +34,6 @@
 #include "QualifiedNameCache.h"
 #include "ThreadTimers.h"
 #include <wtf/MainThread.h>
-#include <wtf/ThreadSpecific.h>
 #include <wtf/Threading.h>
 #include <wtf/text/StringImpl.h>
 
@@ -53,21 +52,9 @@ ThreadGlobalData::~ThreadGlobalData() = default;
 void ThreadGlobalData::destroy()
 {
     m_destroyed = true;
-
-    PAL::ThreadGlobalData::destroy();
-
-    // The ThreadGlobalData destructor is called under the TLS destruction
-    // callback, which is later than when the static atom table is destroyed.
-    // To avoid AtomStrings being destroyed after the table, we clear objects
-    // that have AtomStrings in them.
-    m_eventNames = nullptr;
-    m_threadTimers = nullptr;
-    m_qualifiedNameCache = nullptr;
-    m_fontCache = nullptr;
 }
 
 #if USE(WEB_THREAD)
-static ThreadSpecific<RefPtr<ThreadGlobalData>>* staticData { nullptr };
 static ThreadGlobalData* sharedMainThreadStaticData { nullptr };
 
 void ThreadGlobalData::setWebCoreThreadData()
@@ -76,42 +63,42 @@ void ThreadGlobalData::setWebCoreThreadData()
     ASSERT(&threadGlobalData() != sharedMainThreadStaticData);
 
     // Set WebThread's ThreadGlobalData object to be the same as the main UI thread.
-    **staticData = adoptRef(sharedMainThreadStaticData);
+    Thread::current().m_clientData = adoptRef(sharedMainThreadStaticData);
 
     ASSERT(&threadGlobalData() == sharedMainThreadStaticData);
 }
 
 ThreadGlobalData& threadGlobalData()
 {
-    if (UNLIKELY(!staticData)) {
-        staticData = new ThreadSpecific<RefPtr<ThreadGlobalData>>;
-        auto& result = **staticData;
-        ASSERT(!result);
-        result = adoptRef(new ThreadGlobalData);
-        // WebThread and main UI thread need to share the same object. Save it in a static
-        // here, the WebThread will pick it up in setWebCoreThreadData().
-        if (pthread_main_np()) {
-            sharedMainThreadStaticData = result.get();
-            result->ref();
-        }
-        return *result;
+    auto& thread = Thread::current();
+    auto* clientData = thread.m_clientData.get();
+    if (LIKELY(clientData))
+        return *static_cast<ThreadGlobalData*>(clientData);
+
+    auto data = adoptRef(*new ThreadGlobalData);
+    if (pthread_main_np()) {
+        sharedMainThreadStaticData = data.ptr();
+        data->ref();
     }
 
-    auto& result = **staticData;
-    if (!result)
-        result = adoptRef(new ThreadGlobalData);
-    return *result;
+    clientData = data.ptr();
+    thread.m_clientData = WTFMove(data);
+    return *static_cast<ThreadGlobalData*>(clientData);
 }
 
 #else
 
-static ThreadSpecific<ThreadGlobalData>* staticData { nullptr };
-
 ThreadGlobalData& threadGlobalData()
 {
-    if (UNLIKELY(!staticData))
-        staticData = new ThreadSpecific<ThreadGlobalData>;
-    return **staticData;
+    auto& thread = Thread::current();
+    auto* clientData = thread.m_clientData.get();
+    if (LIKELY(clientData))
+        return *static_cast<ThreadGlobalData*>(clientData);
+
+    auto data = adoptRef(*new ThreadGlobalData);
+    clientData = data.ptr();
+    thread.m_clientData = WTFMove(data);
+    return *static_cast<ThreadGlobalData*>(clientData);
 }
 
 #endif