[WTF] Thread::create should have Thread::tryCreate
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Dec 2017 10:35:39 +0000 (10:35 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Dec 2017 10:35:39 +0000 (10:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180333

Reviewed by Darin Adler.

Source/JavaScriptCore:

* assembler/testmasm.cpp:
(JSC::run):
* b3/air/testair.cpp:
* b3/testb3.cpp:
(JSC::B3::run):
* jsc.cpp:
(functionDollarAgentStart):

Source/WebCore:

No behavior change.

* bindings/js/GCController.cpp:
(WebCore::GCController::garbageCollectOnAlternateThreadForDebugging):
* platform/audio/ReverbConvolver.cpp:
(WebCore::ReverbConvolver::ReverbConvolver):
* platform/audio/ReverbConvolver.h:
* workers/WorkerThread.cpp:
(WebCore::WorkerThread::start):

Source/WebKit:

* UIProcess/API/glib/IconDatabase.cpp:
(WebKit::IconDatabase::open):
* UIProcess/linux/MemoryPressureMonitor.cpp:
(WebKit::MemoryPressureMonitor::MemoryPressureMonitor):

Source/WebKitLegacy:

* Storage/StorageThread.cpp:
(WebCore::StorageThread::start):

Source/WebKitLegacy/win:

* WebKitQuartzCoreAdditions/CVDisplayLink.cpp:
(WKQCA::CVDisplayLink::start):

Source/WTF:

Many callers of Thread::create assume that it returns non-nullptr Thread.
But if the number of threads hits the limit in the system, creating Thread
would fail. In that case, it is really difficult to keep WebKit working.

We introduce Thread::tryCreate, and change the returned value from Thread::create
from RefPtr<Thread> to Ref<Thread>. In Thread::create, we ensure thread creation
succeeds by RELEASE_ASSERT. And we use Thread::create intentionally if the
caller assumes that thread should be created.

* wtf/AutomaticThread.cpp:
(WTF::AutomaticThread::start):
* wtf/ParallelJobsGeneric.cpp:
(WTF::ParallelEnvironment::ThreadPrivate::tryLockFor):
* wtf/Threading.cpp:
(WTF::Thread::tryCreate):
(WTF::Thread::create): Deleted.
* wtf/Threading.h:
(WTF::Thread::create):
* wtf/WorkQueue.cpp:
(WTF::WorkQueue::concurrentApply):

Tools:

* TestWebKitAPI/Tests/WTF/Condition.cpp:
* TestWebKitAPI/Tests/WTF/ParkingLot.cpp:
* TestWebKitAPI/Tests/WTF/Signals.cpp:
(TEST):
* TestWebKitAPI/Tests/WTF/ThreadGroup.cpp:
(TestWebKitAPI::testThreadGroup):
(TestWebKitAPI::TEST):

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

28 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/testmasm.cpp
Source/JavaScriptCore/b3/air/testair.cpp
Source/JavaScriptCore/b3/testb3.cpp
Source/JavaScriptCore/jsc.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/AutomaticThread.cpp
Source/WTF/wtf/ParallelJobsGeneric.cpp
Source/WTF/wtf/Threading.cpp
Source/WTF/wtf/Threading.h
Source/WTF/wtf/WorkQueue.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/GCController.cpp
Source/WebCore/platform/audio/ReverbConvolver.cpp
Source/WebCore/platform/audio/ReverbConvolver.h
Source/WebCore/workers/WorkerThread.cpp
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/glib/IconDatabase.cpp
Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp
Source/WebKitLegacy/ChangeLog
Source/WebKitLegacy/Storage/StorageThread.cpp
Source/WebKitLegacy/win/ChangeLog
Source/WebKitLegacy/win/WebKitQuartzCoreAdditions/CVDisplayLink.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/Condition.cpp
Tools/TestWebKitAPI/Tests/WTF/ParkingLot.cpp
Tools/TestWebKitAPI/Tests/WTF/Signals.cpp
Tools/TestWebKitAPI/Tests/WTF/ThreadGroup.cpp

index fb04028..820ee99 100644 (file)
@@ -1,3 +1,18 @@
+2017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Thread::create should have Thread::tryCreate
+        https://bugs.webkit.org/show_bug.cgi?id=180333
+
+        Reviewed by Darin Adler.
+
+        * assembler/testmasm.cpp:
+        (JSC::run):
+        * b3/air/testair.cpp:
+        * b3/testb3.cpp:
+        (JSC::B3::run):
+        * jsc.cpp:
+        (functionDollarAgentStart):
+
 2017-12-11  Michael Saboff  <msaboff@apple.com>
 
         REGRESSION(r225683): Chakra test failure in es6/regex-unicode.js for 32bit builds
index 80211f7..7d9cebf 100644 (file)
@@ -808,7 +808,7 @@ void run(const char* filter)
 
     Lock lock;
 
-    Vector<RefPtr<Thread>> threads;
+    Vector<Ref<Thread>> threads;
     for (unsigned i = filter ? 1 : WTF::numberOfProcessorCores(); i--;) {
         threads.append(
             Thread::create(
@@ -828,7 +828,7 @@ void run(const char* filter)
                 }));
     }
 
-    for (RefPtr<Thread> thread : threads)
+    for (auto& thread : threads)
         thread->waitForCompletion();
     crashLock.lock();
     dataLog("Completed ", numberOfTests, " tests\n");
index 24bdc2c..477e737 100644 (file)
@@ -2025,7 +2025,7 @@ void run(const char* filter)
 
     Lock lock;
 
-    Vector<RefPtr<Thread>> threads;
+    Vector<Ref<Thread>> threads;
     for (unsigned i = filter ? 1 : WTF::numberOfProcessorCores(); i--;) {
         threads.append(
             Thread::create(
@@ -2045,7 +2045,7 @@ void run(const char* filter)
                 }));
     }
 
-    for (RefPtr<Thread> thread : threads)
+    for (auto& thread : threads)
         thread->waitForCompletion();
     crashLock.lock();
 }
index a0c7e9f..0b22906 100644 (file)
@@ -17794,7 +17794,7 @@ void run(const char* filter)
 
     Lock lock;
 
-    Vector<RefPtr<Thread>> threads;
+    Vector<Ref<Thread>> threads;
     for (unsigned i = filter ? 1 : WTF::numberOfProcessorCores(); i--;) {
         threads.append(
             Thread::create(
@@ -17814,7 +17814,7 @@ void run(const char* filter)
                 }));
     }
 
-    for (RefPtr<Thread> thread : threads)
+    for (auto& thread : threads)
         thread->waitForCompletion();
     crashLock.lock();
 }
index d115e93..c6de07f 100644 (file)
@@ -1560,7 +1560,7 @@ EncodedJSValue JSC_HOST_CALL functionDollarAgentStart(ExecState* exec)
     Condition didStartCondition;
     bool didStart = false;
     
-    RefPtr<Thread> thread = Thread::create(
+    Thread::create(
         "JSC Agent",
         [sourceCode, &didStartLock, &didStartCondition, &didStart] () {
             CommandLine commandLine(0, nullptr);
@@ -1586,8 +1586,7 @@ EncodedJSValue JSC_HOST_CALL functionDollarAgentStart(ExecState* exec)
                         exit(1);
                     return success;
                 });
-        });
-    thread->detach();
+        })->detach();
     
     {
         auto locker = holdLock(didStartLock);
index 1880d86..3d2533b 100644 (file)
@@ -1,3 +1,31 @@
+2017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Thread::create should have Thread::tryCreate
+        https://bugs.webkit.org/show_bug.cgi?id=180333
+
+        Reviewed by Darin Adler.
+
+        Many callers of Thread::create assume that it returns non-nullptr Thread.
+        But if the number of threads hits the limit in the system, creating Thread
+        would fail. In that case, it is really difficult to keep WebKit working.
+
+        We introduce Thread::tryCreate, and change the returned value from Thread::create
+        from RefPtr<Thread> to Ref<Thread>. In Thread::create, we ensure thread creation
+        succeeds by RELEASE_ASSERT. And we use Thread::create intentionally if the
+        caller assumes that thread should be created.
+
+        * wtf/AutomaticThread.cpp:
+        (WTF::AutomaticThread::start):
+        * wtf/ParallelJobsGeneric.cpp:
+        (WTF::ParallelEnvironment::ThreadPrivate::tryLockFor):
+        * wtf/Threading.cpp:
+        (WTF::Thread::tryCreate):
+        (WTF::Thread::create): Deleted.
+        * wtf/Threading.h:
+        (WTF::Thread::create):
+        * wtf/WorkQueue.cpp:
+        (WTF::WorkQueue::concurrentApply):
+
 2017-12-11  Eric Carlson  <eric.carlson@apple.com>
 
         Web Inspector: Optionally log WebKit log parameters as JSON
index 690c80f..2c486c4 100644 (file)
@@ -161,7 +161,7 @@ void AutomaticThread::start(const AbstractLocker&)
     
     m_hasUnderlyingThread = true;
     
-    RefPtr<Thread> thread = Thread::create(
+    Thread::create(
         "WTF::AutomaticThread",
         [=] () {
             if (verbose)
@@ -226,8 +226,7 @@ void AutomaticThread::start(const AbstractLocker&)
                 }
                 RELEASE_ASSERT(result == WorkResult::Continue);
             }
-        });
-    thread->detach();
+        })->detach();
 }
 
 void AutomaticThread::threadDidStart()
index aacc7cc..ae6c17f 100644 (file)
@@ -94,7 +94,7 @@ bool ParallelEnvironment::ThreadPrivate::tryLockFor(ParallelEnvironment* parent)
     }
 
     if (!m_thread) {
-        m_thread = Thread::create("Parallel worker", [this] {
+        m_thread = Thread::tryCreate("Parallel worker", [this] {
             LockHolder lock(m_mutex);
 
             while (m_thread) {
index 84e0579..1620200 100644 (file)
@@ -129,7 +129,7 @@ void Thread::entryPoint(NewThreadContext* newThreadContext)
     function();
 }
 
-RefPtr<Thread> Thread::create(const char* name, Function<void()>&& entryPoint)
+RefPtr<Thread> Thread::tryCreate(const char* name, Function<void()>&& entryPoint)
 {
     WTF::initializeThreading();
     Ref<Thread> thread = adoptRef(*new Thread());
index d195d18..be8274a 100644 (file)
@@ -86,7 +86,13 @@ public:
 
     // Returns nullptr if thread creation failed.
     // The thread name must be a literal since on some platforms it's passed in to the thread.
-    WTF_EXPORT_PRIVATE static RefPtr<Thread> create(const char* threadName, Function<void()>&&);
+    WTF_EXPORT_PRIVATE static RefPtr<Thread> tryCreate(const char* threadName, Function<void()>&&);
+    static inline Ref<Thread> create(const char* threadName, Function<void()>&& function)
+    {
+        auto thread = tryCreate(threadName, WTFMove(function));
+        RELEASE_ASSERT(thread);
+        return thread.releaseNonNull();
+    }
 
     // Returns Thread object.
     static Thread& current();
index 1154a40..063d72c 100644 (file)
@@ -119,7 +119,7 @@ void WorkQueue::concurrentApply(size_t iterations, WTF::Function<void (size_t in
         Condition m_condition;
         Deque<const WTF::Function<void ()>*> m_queue;
 
-        Vector<RefPtr<Thread>> m_workers;
+        Vector<Ref<Thread>> m_workers;
     };
 
     static LazyNeverDestroyed<ThreadPool> threadPool;
index deaeea8..e01765e 100644 (file)
@@ -1,3 +1,20 @@
+2017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Thread::create should have Thread::tryCreate
+        https://bugs.webkit.org/show_bug.cgi?id=180333
+
+        Reviewed by Darin Adler.
+
+        No behavior change.
+
+        * bindings/js/GCController.cpp:
+        (WebCore::GCController::garbageCollectOnAlternateThreadForDebugging):
+        * platform/audio/ReverbConvolver.cpp:
+        (WebCore::ReverbConvolver::ReverbConvolver):
+        * platform/audio/ReverbConvolver.h:
+        * workers/WorkerThread.cpp:
+        (WebCore::WorkerThread::start):
+
 2017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Automatic minimum size is not clamped if min track sizing function is auto
index 37fe31b..065267b 100644 (file)
@@ -101,7 +101,7 @@ void GCController::garbageCollectNowIfNotDoneRecently()
 
 void GCController::garbageCollectOnAlternateThreadForDebugging(bool waitUntilDone)
 {
-    RefPtr<Thread> thread = Thread::create("WebCore: GCController", &collect);
+    auto thread = Thread::create("WebCore: GCController", &collect);
 
     if (waitUntilDone) {
         thread->waitForCompletion();
index 0e74661..145df81 100644 (file)
@@ -61,9 +61,6 @@ ReverbConvolver::ReverbConvolver(AudioChannel* impulseResponse, size_t renderSli
     , m_minFFTSize(MinFFTSize) // First stage will have this size - successive stages will double in size each time
     , m_maxFFTSize(maxFFTSize) // until we hit m_maxFFTSize
     , m_useBackgroundThreads(useBackgroundThreads)
-    , m_backgroundThread(0)
-    , m_wantsToExit(false)
-    , m_moreInputBuffered(false)
 {
     // If we are using background threads then don't exceed this FFT size for the
     // stages which run in the real-time thread.  This avoids having only one or two
index 8ac6eec..d41f3c9 100644 (file)
@@ -84,8 +84,8 @@ private:
     // Background thread and synchronization
     bool m_useBackgroundThreads;
     RefPtr<Thread> m_backgroundThread;
-    bool m_wantsToExit;
-    bool m_moreInputBuffered;
+    bool m_wantsToExit { false };
+    bool m_moreInputBuffered { false };
     mutable Lock m_backgroundThreadMutex;
     mutable Condition m_backgroundThreadConditionVariable;
 };
index f455588..05ff924 100644 (file)
@@ -140,7 +140,7 @@ bool WorkerThread::start(WTF::Function<void(const String&)>&& evaluateCallback)
 
     m_evaluateCallback = WTFMove(evaluateCallback);
 
-    m_thread = Thread::create("WebCore: Worker", [this] {
+    m_thread = Thread::tryCreate("WebCore: Worker", [this] {
         workerThread();
     });
 
index a2e9804..df05878 100644 (file)
@@ -1,3 +1,15 @@
+2017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Thread::create should have Thread::tryCreate
+        https://bugs.webkit.org/show_bug.cgi?id=180333
+
+        Reviewed by Darin Adler.
+
+        * UIProcess/API/glib/IconDatabase.cpp:
+        (WebKit::IconDatabase::open):
+        * UIProcess/linux/MemoryPressureMonitor.cpp:
+        (WebKit::MemoryPressureMonitor::MemoryPressureMonitor):
+
 2017-12-11  Zan Dobersek  <zdobersek@igalia.com>
 
         [CoordGraphics] Move UpdateAtlas, AreaAllocator into the platform layer
index 07eedcb..7c01863 100644 (file)
@@ -219,7 +219,7 @@ bool IconDatabase::open(const String& directory, const String& filename)
     // Lock here as well as first thing in the thread so the thread doesn't actually commence until the createThread() call
     // completes and m_syncThreadRunning is properly set
     m_syncLock.lock();
-    m_syncThread = Thread::create("WebCore: IconDatabase", [this] {
+    m_syncThread = Thread::tryCreate("WebCore: IconDatabase", [this] {
         iconDatabaseSyncThread();
     });
     m_syncThreadRunning = m_syncThread;
index 48d1069..a6baba4 100644 (file)
@@ -248,7 +248,7 @@ MemoryPressureMonitor::MemoryPressureMonitor()
     if (m_eventFD == -1)
         return;
 
-    RefPtr<Thread> thread = Thread::create("MemoryPressureMonitor", [this] {
+    Thread::create("MemoryPressureMonitor", [this] {
         double pollInterval = s_maxPollingIntervalInSeconds;
         while (true) {
             sleep(pollInterval);
@@ -270,8 +270,7 @@ MemoryPressureMonitor::MemoryPressureMonitor()
             pollInterval = pollIntervalForUsedMemoryPercentage(usedPercentage);
         }
         close(m_eventFD);
-    });
-    thread->detach();
+    })->detach();
 }
 
 IPC::Attachment MemoryPressureMonitor::createHandle() const
index bbdfa90..86f6f06 100644 (file)
@@ -1,3 +1,13 @@
+2017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Thread::create should have Thread::tryCreate
+        https://bugs.webkit.org/show_bug.cgi?id=180333
+
+        Reviewed by Darin Adler.
+
+        * Storage/StorageThread.cpp:
+        (WebCore::StorageThread::start):
+
 2017-12-05  Stephan Szabo  <stephan.szabo@sony.com>
 
         Switch windows build to Visual Studio 2017
index c772ff9..94536bb 100644 (file)
@@ -54,7 +54,7 @@ bool StorageThread::start()
 {
     ASSERT(isMainThread());
     if (!m_thread) {
-        m_thread = Thread::create("WebCore: LocalStorage", [this] {
+        m_thread = Thread::tryCreate("WebCore: LocalStorage", [this] {
             threadEntryPoint();
         });
     }
index db28ec1..4ca42b0 100644 (file)
@@ -1,3 +1,13 @@
+2017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Thread::create should have Thread::tryCreate
+        https://bugs.webkit.org/show_bug.cgi?id=180333
+
+        Reviewed by Darin Adler.
+
+        * WebKitQuartzCoreAdditions/CVDisplayLink.cpp:
+        (WKQCA::CVDisplayLink::start):
+
 2017-12-08  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Use StaticLock and Lock instead of Mutex in Windows WebKitLegacy
index 5068af1..b154824 100644 (file)
@@ -64,7 +64,6 @@ void CVDisplayLink::start()
     m_ioThread = Thread::create("WKQCA: CVDisplayLink", [this] {
         runIOThread();
     });
-    ASSERT(m_ioThread);
 }
 
 void CVDisplayLink::stop()
index 5c5daab..129147b 100644 (file)
@@ -1,3 +1,18 @@
+2017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [WTF] Thread::create should have Thread::tryCreate
+        https://bugs.webkit.org/show_bug.cgi?id=180333
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WTF/Condition.cpp:
+        * TestWebKitAPI/Tests/WTF/ParkingLot.cpp:
+        * TestWebKitAPI/Tests/WTF/Signals.cpp:
+        (TEST):
+        * TestWebKitAPI/Tests/WTF/ThreadGroup.cpp:
+        (TestWebKitAPI::testThreadGroup):
+        (TestWebKitAPI::TEST):
+
 2017-12-11  Basuke Suzuki  <Basuke.Suzuki@sony.com>
 
         [WinCairo] Enable running sharded tests
index 7721636..0a329c2 100644 (file)
@@ -89,14 +89,14 @@ void runTest(
     Condition emptyCondition;
     Condition fullCondition;
 
-    Vector<RefPtr<Thread>> consumerThreads;
-    Vector<RefPtr<Thread>> producerThreads;
+    Vector<Ref<Thread>> consumerThreads;
+    Vector<Ref<Thread>> producerThreads;
 
     Vector<unsigned> received;
     Lock receivedLock;
     
     for (unsigned i = numConsumers; i--;) {
-        RefPtr<Thread> threadIdentifier = Thread::create(
+        consumerThreads.append(Thread::create(
             "Consumer thread",
             [&] () {
                 for (;;) {
@@ -124,14 +124,13 @@ void runTest(
                         received.append(result);
                     }
                 }
-            });
-        consumerThreads.append(threadIdentifier);
+            }));
     }
 
     sleep(delay);
 
     for (unsigned i = numProducers; i--;) {
-        RefPtr<Thread> threadIdentifier = Thread::create(
+        producerThreads.append(Thread::create(
             "Producer Thread",
             [&] () {
                 for (unsigned i = 0; i < numMessagesPerProducer; ++i) {
@@ -151,12 +150,11 @@ void runTest(
                     }
                     notify(notifyStyle, emptyCondition, shouldNotify);
                 }
-            });
-        producerThreads.append(threadIdentifier);
+            }));
     }
 
-    for (RefPtr<Thread> threadIdentifier : producerThreads)
-        threadIdentifier->waitForCompletion();
+    for (auto& thread : producerThreads)
+        thread->waitForCompletion();
 
     {
         std::lock_guard<Lock> locker(lock);
@@ -164,8 +162,8 @@ void runTest(
     }
     emptyCondition.notifyAll();
 
-    for (RefPtr<Thread> threadIdentifier : consumerThreads)
-        threadIdentifier->waitForCompletion();
+    for (auto& thread : consumerThreads)
+        thread->waitForCompletion();
 
     EXPECT_EQ(numProducers * numMessagesPerProducer, received.size());
     std::sort(received.begin(), received.end());
index f9440d0..3c43cde 100644 (file)
@@ -127,8 +127,8 @@ struct SingleLatchTest {
                 condition.wait(locker);
         }
 
-        for (RefPtr<Thread> threadIdentifier : threads)
-            threadIdentifier->waitForCompletion();
+        for (auto& thread : threads)
+            thread->waitForCompletion();
     }
 
     // Semaphore operations.
@@ -178,7 +178,7 @@ struct SingleLatchTest {
     std::mutex lock;
     std::condition_variable condition;
     HashSet<Ref<Thread>> awake;
-    Vector<RefPtr<Thread>> threads;
+    Vector<Ref<Thread>> threads;
     RefPtr<Thread> lastAwoken;
 };
 
index 0775a82..373736e 100644 (file)
@@ -47,19 +47,18 @@ TEST(Signals, SignalsWorkOnExit)
     });
 
     Atomic<bool> receiverShouldKeepRunning(true);
-    RefPtr<Thread> receiverThread = (Thread::create("ThreadMessage receiver",
+    Ref<Thread> receiverThread = (Thread::create("ThreadMessage receiver",
         [&receiverShouldKeepRunning] () {
             while (receiverShouldKeepRunning.load()) { }
     }));
-    ASSERT_TRUE(receiverThread);
 
     bool signalFired;
     {
-        std::unique_lock<std::mutex> locker(static_cast<ReflectedThread*>(receiverThread.get())->m_mutex);
+        std::unique_lock<std::mutex> locker(static_cast<ReflectedThread&>(receiverThread.get()).m_mutex);
         receiverShouldKeepRunning.store(false);
-        EXPECT_FALSE(static_cast<ReflectedThread*>(receiverThread.get())->hasExited());
+        EXPECT_FALSE(static_cast<ReflectedThread&>(receiverThread.get()).hasExited());
         sleep(1);
-        signalFired = !pthread_kill(static_cast<ReflectedThread*>(receiverThread.get())->m_handle, std::get<0>(toSystemSignal(Signal::Usr)));
+        signalFired = !pthread_kill(static_cast<ReflectedThread&>(receiverThread.get()).m_handle, std::get<0>(toSystemSignal(Signal::Usr)));
     }
 
     receiverThread->waitForCompletion();
index b7a188a..7551071 100644 (file)
@@ -41,12 +41,12 @@ static void testThreadGroup(Mode mode)
     Lock lock;
     Condition condition;
     Condition restartCondition;
-    Vector<RefPtr<Thread>> threads;
+    Vector<Ref<Thread>> threads;
 
     {
         auto locker = holdLock(lock);
         for (unsigned i = 0; i < numberOfThreads; ++i) {
-            RefPtr<Thread> thread = Thread::create("ThreadGroupWorker", [&] {
+            Ref<Thread> thread = Thread::create("ThreadGroupWorker", [&] {
                 auto locker = holdLock(lock);
                 if (mode == Mode::AddCurrentThread)
                     threadGroup->addCurrentThread();
@@ -57,8 +57,8 @@ static void testThreadGroup(Mode mode)
                 });
             });
             if (mode == Mode::Add)
-                EXPECT_TRUE(threadGroup->add(*thread) == ThreadGroupAddResult::NewlyAdded);
-            threads.append(thread);
+                EXPECT_TRUE(threadGroup->add(thread.get()) == ThreadGroupAddResult::NewlyAdded);
+            threads.append(WTFMove(thread));
         }
 
         condition.wait(lock, [&] {
@@ -102,9 +102,9 @@ TEST(WTF, ThreadGroupAddCurrentThread)
 TEST(WTF, ThreadGroupDoNotAddDeadThread)
 {
     std::shared_ptr<ThreadGroup> threadGroup = ThreadGroup::create();
-    RefPtr<Thread> thread = Thread::create("ThreadGroupWorker", [&] { });
+    Ref<Thread> thread = Thread::create("ThreadGroupWorker", [&] { });
     thread->waitForCompletion();
-    EXPECT_TRUE(threadGroup->add(*thread) == ThreadGroupAddResult::NotAdded);
+    EXPECT_TRUE(threadGroup->add(thread.get()) == ThreadGroupAddResult::NotAdded);
 
     auto threadGroupLocker = holdLock(threadGroup->getLock());
     EXPECT_EQ(threadGroup->threads(threadGroupLocker).size(), 0u);
@@ -116,14 +116,14 @@ TEST(WTF, ThreadGroupAddDuplicateThreads)
     Lock lock;
     Condition restartCondition;
     std::shared_ptr<ThreadGroup> threadGroup = ThreadGroup::create();
-    RefPtr<Thread> thread = Thread::create("ThreadGroupWorker", [&] {
+    Ref<Thread> thread = Thread::create("ThreadGroupWorker", [&] {
         auto locker = holdLock(lock);
         restartCondition.wait(lock, [&] {
             return restarting;
         });
     });
-    EXPECT_TRUE(threadGroup->add(*thread) == ThreadGroupAddResult::NewlyAdded);
-    EXPECT_TRUE(threadGroup->add(*thread) == ThreadGroupAddResult::AlreadyAdded);
+    EXPECT_TRUE(threadGroup->add(thread.get()) == ThreadGroupAddResult::NewlyAdded);
+    EXPECT_TRUE(threadGroup->add(thread.get()) == ThreadGroupAddResult::AlreadyAdded);
 
     {
         auto threadGroupLocker = holdLock(threadGroup->getLock());