Changeset 225778 in webkit


Ignore:
Timestamp:
Dec 12, 2017 2:35:39 AM (6 years ago)
Author:
Yusuke Suzuki
Message:

[WTF] Thread::create should have Thread::tryCreate
​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):

Location:
trunk
Files:
28 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r225771 r225778  
     12017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] Thread::create should have Thread::tryCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=180333
     5
     6        Reviewed by Darin Adler.
     7
     8        * assembler/testmasm.cpp:
     9        (JSC::run):
     10        * b3/air/testair.cpp:
     11        * b3/testb3.cpp:
     12        (JSC::B3::run):
     13        * jsc.cpp:
     14        (functionDollarAgentStart):
     15
    1162017-12-11  Michael Saboff  <msaboff@apple.com>
    217

  • trunk/Source/JavaScriptCore/assembler/testmasm.cpp

    r225536 r225778  
    809809    Lock lock;
    810810
    811     Vector<RefPtr<Thread>> threads;
     811    Vector<Ref<Thread>> threads;
    812812    for (unsigned i = filter ? 1 : WTF::numberOfProcessorCores(); i--;) {
    813813        threads.append(
    … …  
    829829    }
    830830
    831     for (RefPtr<Thread> thread : threads)
     831    for (auto& thread : threads)
    832832        thread->waitForCompletion();
    833833    crashLock.lock();

  • trunk/Source/JavaScriptCore/b3/air/testair.cpp

    r224550 r225778  
    20262026    Lock lock;
    20272027
    2028     Vector<RefPtr<Thread>> threads;
     2028    Vector<Ref<Thread>> threads;
    20292029    for (unsigned i = filter ? 1 : WTF::numberOfProcessorCores(); i--;) {
    20302030        threads.append(
    … …  
    20462046    }
    20472047
    2048     for (RefPtr<Thread> thread : threads)
     2048    for (auto& thread : threads)
    20492049        thread->waitForCompletion();
    20502050    crashLock.lock();

  • trunk/Source/JavaScriptCore/b3/testb3.cpp

    r225659 r225778  
    1779517795    Lock lock;
    1779617796
    17797     Vector<RefPtr<Thread>> threads;
     17797    Vector<Ref<Thread>> threads;
    1779817798    for (unsigned i = filter ? 1 : WTF::numberOfProcessorCores(); i--;) {
    1779917799        threads.append(
    … …  
    1781517815    }
    1781617816
    17817     for (RefPtr<Thread> thread : threads)
     17817    for (auto& thread : threads)
    1781817818        thread->waitForCompletion();
    1781917819    crashLock.lock();

  • trunk/Source/JavaScriptCore/jsc.cpp

    r225352 r225778  
    15611561    bool didStart = false;
    15621562   
    1563     RefPtr<Thread> thread = Thread::create(
     1563    Thread::create(
    15641564        "JSC Agent",
    15651565        [sourceCode, &didStartLock, &didStartCondition, &didStart] () {
    … …  
    15871587                    return success;
    15881588                });
    1589         });
    1590     thread->detach();
     1589        })->detach();
    15911590   
    15921591    {

  • trunk/Source/WTF/ChangeLog

    r225764 r225778  
     12017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] Thread::create should have Thread::tryCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=180333
     5
     6        Reviewed by Darin Adler.
     7
     8        Many callers of Thread::create assume that it returns non-nullptr Thread.
     9        But if the number of threads hits the limit in the system, creating Thread
     10        would fail. In that case, it is really difficult to keep WebKit working.
     11
     12        We introduce Thread::tryCreate, and change the returned value from Thread::create
     13        from RefPtr<Thread> to Ref<Thread>. In Thread::create, we ensure thread creation
     14        succeeds by RELEASE_ASSERT. And we use Thread::create intentionally if the
     15        caller assumes that thread should be created.
     16
     17        * wtf/AutomaticThread.cpp:
     18        (WTF::AutomaticThread::start):
     19        * wtf/ParallelJobsGeneric.cpp:
     20        (WTF::ParallelEnvironment::ThreadPrivate::tryLockFor):
     21        * wtf/Threading.cpp:
     22        (WTF::Thread::tryCreate):
     23        (WTF::Thread::create): Deleted.
     24        * wtf/Threading.h:
     25        (WTF::Thread::create):
     26        * wtf/WorkQueue.cpp:
     27        (WTF::WorkQueue::concurrentApply):
     28
    1292017-12-11  Eric Carlson  <eric.carlson@apple.com>
    230

  • trunk/Source/WTF/wtf/AutomaticThread.cpp

    r219653 r225778  
    162162    m_hasUnderlyingThread = true;
    163163   
    164     RefPtr<Thread> thread = Thread::create(
     164    Thread::create(
    165165        "WTF::AutomaticThread",
    166166        [=] () {
    … …  
    227227                RELEASE_ASSERT(result == WorkResult::Continue);
    228228            }
    229         });
    230     thread->detach();
     229        })->detach();
    231230}
    232231

  • trunk/Source/WTF/wtf/ParallelJobsGeneric.cpp

    r218816 r225778  
    9595
    9696    if (!m_thread) {
    97         m_thread = Thread::create("Parallel worker", [this] {
     97        m_thread = Thread::tryCreate("Parallel worker", [this] {
    9898            LockHolder lock(m_mutex);
    9999

  • trunk/Source/WTF/wtf/Threading.cpp

    r225530 r225778  
    130130}
    131131
    132 RefPtr<Thread> Thread::create(const char* name, Function<void()>&& entryPoint)
     132RefPtr<Thread> Thread::tryCreate(const char* name, Function<void()>&& entryPoint)
    133133{
    134134    WTF::initializeThreading();

  • trunk/Source/WTF/wtf/Threading.h

    r225735 r225778  
    8787    // Returns nullptr if thread creation failed.
    8888    // The thread name must be a literal since on some platforms it's passed in to the thread.
    89     WTF_EXPORT_PRIVATE static RefPtr<Thread> create(const char* threadName, Function<void()>&&);
     89    WTF_EXPORT_PRIVATE static RefPtr<Thread> tryCreate(const char* threadName, Function<void()>&&);
     90    static inline Ref<Thread> create(const char* threadName, Function<void()>&& function)
     91    {
     92        auto thread = tryCreate(threadName, WTFMove(function));
     93        RELEASE_ASSERT(thread);
     94        return thread.releaseNonNull();
     95    }
    9096
    9197    // Returns Thread object.

  • trunk/Source/WTF/wtf/WorkQueue.cpp

    r225681 r225778  
    120120        Deque<const WTF::Function<void ()>*> m_queue;
    121121
    122         Vector<RefPtr<Thread>> m_workers;
     122        Vector<Ref<Thread>> m_workers;
    123123    };
    124124

  • trunk/Source/WebCore/ChangeLog

    r225776 r225778  
     12017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] Thread::create should have Thread::tryCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=180333
     5
     6        Reviewed by Darin Adler.
     7
     8        No behavior change.
     9
     10        * bindings/js/GCController.cpp:
     11        (WebCore::GCController::garbageCollectOnAlternateThreadForDebugging):
     12        * platform/audio/ReverbConvolver.cpp:
     13        (WebCore::ReverbConvolver::ReverbConvolver):
     14        * platform/audio/ReverbConvolver.h:
     15        * workers/WorkerThread.cpp:
     16        (WebCore::WorkerThread::start):
     17
    1182017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
    219

  • trunk/Source/WebCore/bindings/js/GCController.cpp

    r223476 r225778  
    102102void GCController::garbageCollectOnAlternateThreadForDebugging(bool waitUntilDone)
    103103{
    104     RefPtr<Thread> thread = Thread::create("WebCore: GCController", &collect);
     104    auto thread = Thread::create("WebCore: GCController", &collect);
    105105
    106106    if (waitUntilDone) {

  • trunk/Source/WebCore/platform/audio/ReverbConvolver.cpp

    r218816 r225778  
    6262    , m_maxFFTSize(maxFFTSize) // until we hit m_maxFFTSize
    6363    , m_useBackgroundThreads(useBackgroundThreads)
    64     , m_backgroundThread(0)
    65     , m_wantsToExit(false)
    66     , m_moreInputBuffered(false)
    6764{
    6865    // If we are using background threads then don't exceed this FFT size for the

  • trunk/Source/WebCore/platform/audio/ReverbConvolver.h

    r218816 r225778  
    8585    bool m_useBackgroundThreads;
    8686    RefPtr<Thread> m_backgroundThread;
    87     bool m_wantsToExit;
    88     bool m_moreInputBuffered;
     87    bool m_wantsToExit { false };
     88    bool m_moreInputBuffered { false };
    8989    mutable Lock m_backgroundThreadMutex;
    9090    mutable Condition m_backgroundThreadConditionVariable;

  • trunk/Source/WebCore/workers/WorkerThread.cpp

    r225343 r225778  
    141141    m_evaluateCallback = WTFMove(evaluateCallback);
    142142
    143     m_thread = Thread::create("WebCore: Worker", [this] {
     143    m_thread = Thread::tryCreate("WebCore: Worker", [this] {
    144144        workerThread();
    145145    });

  • trunk/Source/WebKit/ChangeLog

    r225773 r225778  
     12017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] Thread::create should have Thread::tryCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=180333
     5
     6        Reviewed by Darin Adler.
     7
     8        * UIProcess/API/glib/IconDatabase.cpp:
     9        (WebKit::IconDatabase::open):
     10        * UIProcess/linux/MemoryPressureMonitor.cpp:
     11        (WebKit::MemoryPressureMonitor::MemoryPressureMonitor):
     12
    1132017-12-11  Zan Dobersek  <zdobersek@igalia.com>
    214

  • trunk/Source/WebKit/UIProcess/API/glib/IconDatabase.cpp

    r225470 r225778  
    220220    // completes and m_syncThreadRunning is properly set
    221221    m_syncLock.lock();
    222     m_syncThread = Thread::create("WebCore: IconDatabase", [this] {
     222    m_syncThread = Thread::tryCreate("WebCore: IconDatabase", [this] {
    223223        iconDatabaseSyncThread();
    224224    });

  • trunk/Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp

    r215265 r225778  
    249249        return;
    250250
    251     RefPtr<Thread> thread = Thread::create("MemoryPressureMonitor", [this] {
     251    Thread::create("MemoryPressureMonitor", [this] {
    252252        double pollInterval = s_maxPollingIntervalInSeconds;
    253253        while (true) {
    … …  
    271271        }
    272272        close(m_eventFD);
    273     });
    274     thread->detach();
     273    })->detach();
    275274}
    276275

  • trunk/Source/WebKitLegacy/ChangeLog

    r225563 r225778  
     12017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] Thread::create should have Thread::tryCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=180333
     5
     6        Reviewed by Darin Adler.
     7
     8        * Storage/StorageThread.cpp:
     9        (WebCore::StorageThread::start):
     10
    1112017-12-05  Stephan Szabo  <stephan.szabo@sony.com>
    212

  • trunk/Source/WebKitLegacy/Storage/StorageThread.cpp

    r219595 r225778  
    5555    ASSERT(isMainThread());
    5656    if (!m_thread) {
    57         m_thread = Thread::create("WebCore: LocalStorage", [this] {
     57        m_thread = Thread::tryCreate("WebCore: LocalStorage", [this] {
    5858            threadEntryPoint();
    5959        });

  • trunk/Source/WebKitLegacy/win/ChangeLog

    r225672 r225778  
     12017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] Thread::create should have Thread::tryCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=180333
     5
     6        Reviewed by Darin Adler.
     7
     8        * WebKitQuartzCoreAdditions/CVDisplayLink.cpp:
     9        (WKQCA::CVDisplayLink::start):
     10
    1112017-12-08  Yusuke Suzuki  <utatane.tea@gmail.com>
    212

  • trunk/Source/WebKitLegacy/win/WebKitQuartzCoreAdditions/CVDisplayLink.cpp

    r224629 r225778  
    6565        runIOThread();
    6666    });
    67     ASSERT(m_ioThread);
    6867}
    6968

  • trunk/Tools/ChangeLog

    r225770 r225778  
     12017-12-12  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [WTF] Thread::create should have Thread::tryCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=180333
     5
     6        Reviewed by Darin Adler.
     7
     8        * TestWebKitAPI/Tests/WTF/Condition.cpp:
     9        * TestWebKitAPI/Tests/WTF/ParkingLot.cpp:
     10        * TestWebKitAPI/Tests/WTF/Signals.cpp:
     11        (TEST):
     12        * TestWebKitAPI/Tests/WTF/ThreadGroup.cpp:
     13        (TestWebKitAPI::testThreadGroup):
     14        (TestWebKitAPI::TEST):
     15
    1162017-12-11  Basuke Suzuki  <Basuke.Suzuki@sony.com>
    217

  • trunk/Tools/TestWebKitAPI/Tests/WTF/Condition.cpp

    r215265 r225778  
    9090    Condition fullCondition;
    9191
    92     Vector<RefPtr<Thread>> consumerThreads;
    93     Vector<RefPtr<Thread>> producerThreads;
     92    Vector<Ref<Thread>> consumerThreads;
     93    Vector<Ref<Thread>> producerThreads;
    9494
    9595    Vector<unsigned> received;
    … …  
    9797   
    9898    for (unsigned i = numConsumers; i--;) {
    99         RefPtr<Thread> threadIdentifier = Thread::create(
     99        consumerThreads.append(Thread::create(
    100100            "Consumer thread",
    101101            [&] () {
    … …  
    125125                    }
    126126                }
    127             });
    128         consumerThreads.append(threadIdentifier);
     127            }));
    129128    }
    130129
    … …  
    132131
    133132    for (unsigned i = numProducers; i--;) {
    134         RefPtr<Thread> threadIdentifier = Thread::create(
     133        producerThreads.append(Thread::create(
    135134            "Producer Thread",
    136135            [&] () {
    … …  
    152151                    notify(notifyStyle, emptyCondition, shouldNotify);
    153152                }
    154             });
    155         producerThreads.append(threadIdentifier);
    156     }
    157 
    158     for (RefPtr<Thread> threadIdentifier : producerThreads)
    159         threadIdentifier->waitForCompletion();
     153            }));
     154    }
     155
     156    for (auto& thread : producerThreads)
     157        thread->waitForCompletion();
    160158
    161159    {
    … …  
    165163    emptyCondition.notifyAll();
    166164
    167     for (RefPtr<Thread> threadIdentifier : consumerThreads)
    168         threadIdentifier->waitForCompletion();
     165    for (auto& thread : consumerThreads)
     166        thread->waitForCompletion();
    169167
    170168    EXPECT_EQ(numProducers * numMessagesPerProducer, received.size());

  • trunk/Tools/TestWebKitAPI/Tests/WTF/ParkingLot.cpp

    r225470 r225778  
    128128        }
    129129
    130         for (RefPtr<Thread> threadIdentifier : threads)
    131             threadIdentifier->waitForCompletion();
     130        for (auto& thread : threads)
     131            thread->waitForCompletion();
    132132    }
    133133
    … …  
    179179    std::condition_variable condition;
    180180    HashSet<Ref<Thread>> awake;
    181     Vector<RefPtr<Thread>> threads;
     181    Vector<Ref<Thread>> threads;
    182182    RefPtr<Thread> lastAwoken;
    183183};

  • trunk/Tools/TestWebKitAPI/Tests/WTF/Signals.cpp

    r220562 r225778  
    4848
    4949    Atomic<bool> receiverShouldKeepRunning(true);
    50     RefPtr<Thread> receiverThread = (Thread::create("ThreadMessage receiver",
     50    Ref<Thread> receiverThread = (Thread::create("ThreadMessage receiver",
    5151        [&receiverShouldKeepRunning] () {
    5252            while (receiverShouldKeepRunning.load()) { }
    5353    }));
    54     ASSERT_TRUE(receiverThread);
    5554
    5655    bool signalFired;
    5756    {
    58         std::unique_lock<std::mutex> locker(static_cast<ReflectedThread*>(receiverThread.get())->m_mutex);
     57        std::unique_lock<std::mutex> locker(static_cast<ReflectedThread&>(receiverThread.get()).m_mutex);
    5958        receiverShouldKeepRunning.store(false);
    60         EXPECT_FALSE(static_cast<ReflectedThread*>(receiverThread.get())->hasExited());
     59        EXPECT_FALSE(static_cast<ReflectedThread&>(receiverThread.get()).hasExited());
    6160        sleep(1);
    62         signalFired = !pthread_kill(static_cast<ReflectedThread*>(receiverThread.get())->m_handle, std::get<0>(toSystemSignal(Signal::Usr)));
     61        signalFired = !pthread_kill(static_cast<ReflectedThread&>(receiverThread.get()).m_handle, std::get<0>(toSystemSignal(Signal::Usr)));
    6362    }
    6463

  • trunk/Tools/TestWebKitAPI/Tests/WTF/ThreadGroup.cpp

    r219760 r225778  
    4242    Condition condition;
    4343    Condition restartCondition;
    44     Vector<RefPtr<Thread>> threads;
     44    Vector<Ref<Thread>> threads;
    4545
    4646    {
    4747        auto locker = holdLock(lock);
    4848        for (unsigned i = 0; i < numberOfThreads; ++i) {
    49             RefPtr<Thread> thread = Thread::create("ThreadGroupWorker", [&] {
     49            Ref<Thread> thread = Thread::create("ThreadGroupWorker", [&] {
    5050                auto locker = holdLock(lock);
    5151                if (mode == Mode::AddCurrentThread)
    … …  
    5858            });
    5959            if (mode == Mode::Add)
    60                 EXPECT_TRUE(threadGroup->add(*thread) == ThreadGroupAddResult::NewlyAdded);
    61             threads.append(thread);
     60                EXPECT_TRUE(threadGroup->add(thread.get()) == ThreadGroupAddResult::NewlyAdded);
     61            threads.append(WTFMove(thread));
    6262        }
    6363
    … …  
    103103{
    104104    std::shared_ptr<ThreadGroup> threadGroup = ThreadGroup::create();
    105     RefPtr<Thread> thread = Thread::create("ThreadGroupWorker", [&] { });
     105    Ref<Thread> thread = Thread::create("ThreadGroupWorker", [&] { });
    106106    thread->waitForCompletion();
    107     EXPECT_TRUE(threadGroup->add(*thread) == ThreadGroupAddResult::NotAdded);
     107    EXPECT_TRUE(threadGroup->add(thread.get()) == ThreadGroupAddResult::NotAdded);
    108108
    109109    auto threadGroupLocker = holdLock(threadGroup->getLock());
    … …  
    117117    Condition restartCondition;
    118118    std::shared_ptr<ThreadGroup> threadGroup = ThreadGroup::create();
    119     RefPtr<Thread> thread = Thread::create("ThreadGroupWorker", [&] {
     119    Ref<Thread> thread = Thread::create("ThreadGroupWorker", [&] {
    120120        auto locker = holdLock(lock);
    121121        restartCondition.wait(lock, [&] {
    … …  
    123123        });
    124124    });
    125     EXPECT_TRUE(threadGroup->add(*thread) == ThreadGroupAddResult::NewlyAdded);
    126     EXPECT_TRUE(threadGroup->add(*thread) == ThreadGroupAddResult::AlreadyAdded);
     125    EXPECT_TRUE(threadGroup->add(thread.get()) == ThreadGroupAddResult::NewlyAdded);
     126    EXPECT_TRUE(threadGroup->add(thread.get()) == ThreadGroupAddResult::AlreadyAdded);
    127127
    128128    {
Note: See TracChangeset for help on using the changeset viewer.