[WTF] Revert ThreadGroup to std::shared_ptr / std::weak_ptr implement…

…ation

https://bugs.webkit.org/show_bug.cgi?id=260373
rdar://113739113

Reviewed by Mark Lam.

Speculative fix. Let's revert our ThreadGroup and Thread implementation to use std::shared_ptr and std::weak_ptr
instead of WebKit ThreadSafeWeakPtr and ThreadSafeWeakHashSet. This is basically partial revert of 257595@main
and 258267@main (reverting part related to ThreadGroup and Thread).

* Source/JavaScriptCore/heap/MachineStackMarker.h:
* Source/WTF/wtf/ThreadGroup.cpp:
(WTF::ThreadGroup::~ThreadGroup):
(WTF::ThreadGroup::create): Deleted.
* Source/WTF/wtf/ThreadGroup.h:
* Source/WTF/wtf/Threading.cpp:
(WTF::Thread::didExit):
(WTF::Thread::addToThreadGroup):
(WTF::Thread::removeFromThreadGroup):
(WTF::Thread::numberOfThreadGroups):
* Source/WTF/wtf/Threading.h:
* Source/WTF/wtf/linux/RealTimeThreads.h:
* Source/WTF/wtf/threads/Signals.cpp:
(WTF::activeThreads):
* Tools/TestWebKitAPI/Tests/WTF/ThreadGroup.cpp:
(TestWebKitAPI::testThreadGroup):
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/267043@main

Source/JavaScriptCore/heap/MachineStackMarker.h

@@ -59,7 +59,7 @@ class MachineThreads {
     void tryCopyOtherThreadStack(const ThreadSuspendLocker&, Thread&, void*, size_t capacity, size_t*);
     bool tryCopyOtherThreadStacks(const AbstractLocker&, void*, size_t capacity, size_t*, Thread&);
 
-    Ref<ThreadGroup> m_threadGroup;
+    std::shared_ptr<ThreadGroup> m_threadGroup;
 };
 
 #define DECLARE_AND_COMPUTE_CURRENT_THREAD_STATE(stateName) \

Source/WTF/wtf/ThreadGroup.cpp

@@ -28,13 +28,13 @@
 
 namespace WTF {
 
-Ref<ThreadGroup> ThreadGroup::create()
+ThreadGroup::~ThreadGroup()
 {
-    return adoptRef(*new ThreadGroup);
+    Locker locker { m_lock };
+    for (auto& thread : m_threads)
+        thread->removeFromThreadGroup(locker, *this);
 }
 
-ThreadGroup::~ThreadGroup() = default;
-
 ThreadGroupAddResult ThreadGroup::add(const AbstractLocker& locker, Thread& thread)
 {
     return thread.addToThreadGroup(locker, *this);

Source/WTF/wtf/ThreadGroup.h

@@ -34,13 +34,17 @@ namespace WTF {
 
 enum class ThreadGroupAddResult { NewlyAdded, AlreadyAdded, NotAdded };
 
-class ThreadGroup final : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<ThreadGroup> {
+class ThreadGroup final : public std::enable_shared_from_this<ThreadGroup> {
     WTF_MAKE_FAST_ALLOCATED;
     WTF_MAKE_NONCOPYABLE(ThreadGroup);
 public:
     friend class Thread;
 
-    WTF_EXPORT_PRIVATE static Ref<ThreadGroup> create();
+    static std::shared_ptr<ThreadGroup> create()
+    {
+        return std::allocate_shared<ThreadGroup>(FastAllocator<ThreadGroup>());
+    }
+
     WTF_EXPORT_PRIVATE ThreadGroupAddResult add(Thread&);
     WTF_EXPORT_PRIVATE ThreadGroupAddResult add(const AbstractLocker&, Thread&);
     WTF_EXPORT_PRIVATE ThreadGroupAddResult addCurrentThread();
@@ -54,6 +58,11 @@ class ThreadGroup final : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr
     ThreadGroup() = default;
 
 private:
+    std::weak_ptr<ThreadGroup> weakFromThis()
+    {
+        return shared_from_this();
+    }
+
     // We use WordLock since it can be used when deallocating TLS.
     WordLock m_lock;
     ListHashSet<Ref<Thread>> m_threads;

Source/WTF/wtf/Threading.cpp

@@ -316,10 +316,15 @@ void Thread::didExit()
 
     if (shouldRemoveThreadFromThreadGroup()) {
         {
-            Vector<Ref<ThreadGroup>> threadGroups;
+            Vector<std::shared_ptr<ThreadGroup>> threadGroups;
             {
                 Locker locker { m_mutex };
-                threadGroups = m_threadGroups.values();
+                for (auto& threadGroupPointerPair : m_threadGroupMap) {
+                    // If ThreadGroup is just being destroyed,
+                    // we do not need to perform unregistering.
+                    if (auto retained = threadGroupPointerPair.value.lock())
+                        threadGroups.append(WTFMove(retained));
+                }
                 m_isShuttingDown = true;
             }
             for (auto& threadGroup : threadGroups) {
@@ -343,16 +348,25 @@ ThreadGroupAddResult Thread::addToThreadGroup(const AbstractLocker& threadGroupL
     if (m_isShuttingDown)
         return ThreadGroupAddResult::NotAdded;
     if (threadGroup.m_threads.add(*this).isNewEntry) {
-        m_threadGroups.add(threadGroup);
+        m_threadGroupMap.add(&threadGroup, threadGroup.weakFromThis());
         return ThreadGroupAddResult::NewlyAdded;
     }
     return ThreadGroupAddResult::AlreadyAdded;
 }
 
+void Thread::removeFromThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup& threadGroup)
+{
+    UNUSED_PARAM(threadGroupLocker);
+    Locker locker { m_mutex };
+    if (m_isShuttingDown)
+        return;
+    m_threadGroupMap.remove(&threadGroup);
+}
+
 unsigned Thread::numberOfThreadGroups()
 {
     Locker locker { m_mutex };
-    return m_threadGroups.values().size();
+    return m_threadGroupMap.size();
 }
 
 bool Thread::exchangeIsCompilationThread(bool newValue)

Source/WTF/wtf/Threading.h

@@ -333,6 +333,7 @@ class WTF_CAPABILITY("is current") Thread : public ThreadSafeRefCounted<Thread>,
 
     // These functions are only called from ThreadGroup.
     ThreadGroupAddResult addToThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup&);
+    void removeFromThreadGroup(const AbstractLocker& threadGroupLocker, ThreadGroup&);
 
     // For pthread, the Thread instance is ref'ed and held in thread-specific storage. It will be deref'ed by destructTLS at thread destruction time.
     // It employs pthreads-specific 2-pass destruction to reliably remove Thread.
@@ -369,7 +370,7 @@ class WTF_CAPABILITY("is current") Thread : public ThreadSafeRefCounted<Thread>,
     // Use WordLock since WordLock does not depend on ThreadSpecific and this "Thread".
     WordLock m_mutex;
     StackBounds m_stack { StackBounds::emptyBounds() };
-    ThreadSafeWeakHashSet<ThreadGroup> m_threadGroups;
+    HashMap<ThreadGroup*, std::weak_ptr<ThreadGroup>> m_threadGroupMap;
     PlatformThreadHandle m_handle;
     uint32_t m_uid { ++s_uid };
 #if OS(WINDOWS)

Source/WTF/wtf/linux/RealTimeThreads.h

@@ -55,7 +55,7 @@ class RealTimeThreads {
     void discardRealTimeKitProxyTimerFired();
 #endif
 
-    Ref<ThreadGroup> m_threadGroup;
+    std::shared_ptr<ThreadGroup> m_threadGroup;
     bool m_enabled { true };
 #if USE(GLIB)
     std::optional<GRefPtr<GDBusProxy>> m_realTimeKitProxy;

Source/WTF/wtf/threads/Signals.cpp

@@ -316,13 +316,13 @@ inline void setExceptionPorts(const AbstractLocker& threadGroupLocker, Thread& t
 
 static ThreadGroup& activeThreads()
 {
-    static LazyNeverDestroyed<Ref<ThreadGroup>> activeThreads;
+    static LazyNeverDestroyed<std::shared_ptr<ThreadGroup>> activeThreads;
     static std::once_flag initializeKey;
     std::call_once(initializeKey, [&] {
         Config::AssertNotFrozenScope assertScope;
         activeThreads.construct(ThreadGroup::create());
     });
-    return activeThreads.get();
+    return (*activeThreads.get());
 }
 
 void registerThreadForMachExceptionHandling(Thread& thread)

Tools/TestWebKitAPI/Tests/WTF/ThreadGroup.cpp

@@ -163,7 +163,7 @@ TEST(WTF, ThreadGroupRemove)
 
     Vector<Ref<Thread>> threads;
 
-    RefPtr<ThreadGroup> threadGroup = ThreadGroup::create();
+    auto threadGroup = ThreadGroup::create();
     for (unsigned i = 0; i < NumberOfThreads; i++) {
         auto thread = Thread::create("ThreadGroupWorker", [&]() {
             Locker locker { lock };