diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp index 01801a9f7dfb..d19578bf70ac 100644 --- a/Source/JavaScriptCore/heap/Heap.cpp +++ b/Source/JavaScriptCore/heap/Heap.cpp @@ -360,7 +360,10 @@ Heap::Heap(VM& vm, HeapType heapType) , m_sharedMutatorMarkStack(makeUnique()) , m_helperClient(&heapHelperPool()) , m_threadLock(Box::create()) - , m_threadCondition(AutomaticThreadCondition::create()) + // If the OS refuses to create the collector thread (pthread_create EAGAIN under a thread or pid + // limit), the mutator finishes the collection itself. See takeConnBecauseCollectorThreadCouldNotStart() + // and collectAsTheCollectorBecauseCollectorThreadCouldNotStart(). + , m_threadCondition(AutomaticThreadCondition::create(AutomaticThreadCondition::StartFailure::Retry)) // HeapCellTypes , auxiliaryHeapCellType(CellAttributes(DoesNotNeedDestruction, HeapCell::Auxiliary)) @@ -2153,6 +2156,7 @@ NEVER_INLINE void Heap::collectInMutatorThread() template void Heap::waitForCollector(const Func& func) { + bool collectorThreadCouldNotStart = false; for (;;) { bool done; { @@ -2176,12 +2180,22 @@ void Heap::waitForCollector(const Func& func) m_mutatorDidRun = true; // FIXME: We wouldn't need this if stopIfNecessarySlow() had a mode where it knew to just // do the collection. - relinquishConn(); + if (!collectorThreadCouldNotStart) + relinquishConn(); if (done) { clearMutatorWaiting(); // Clean up just in case. return; } + + // We still have the conn if the collector thread could not be created. Nobody else can finish + // this collection, so this thread plays the collector too (see the function). It stops + // offering the conn for the rest of this wait: each offer retries the thread. + if (collectorThreadCouldNotStart || (m_worldState.load() & mutatorHasConnBit)) { + collectorThreadCouldNotStart = true; + collectAsTheCollectorBecauseCollectorThreadCouldNotStart(); + continue; + } // If mutatorWaitingBit is still set then we want to wait. ParkingLot::compareAndPark(&m_worldState, oldState | mutatorWaitingBit); @@ -2264,20 +2278,71 @@ bool Heap::relinquishConn(unsigned oldState) if (!m_worldState.compareExchangeWeak(oldState, oldState & ~mutatorHasConnBit)) return true; // Loop around. - finishRelinquishingConn(); - return true; + // If the conn came back because the collector thread could not be created, we are done: looping + // around would relinquish it again and retry the thread on every iteration. + return finishRelinquishingConn(); } -void Heap::finishRelinquishingConn() +// Returns false if the conn had to be taken back because no collector thread could be created. +bool Heap::finishRelinquishingConn() { dataLogLnIf(HeapInternal::verbose, "Relinquished the conn."); sanitizeStackForVM(vm()); Locker locker { *m_threadLock }; - if (!m_requests.isEmpty()) - m_threadCondition->notifyOne(locker); + bool handedOff = true; + if (!m_requests.isEmpty() && !m_threadCondition->notifyOne(locker)) { + takeConnBecauseCollectorThreadCouldNotStart(locker); + handedOff = false; + } ParkingLot::unparkAll(&m_worldState); + return handedOff; +} + +// The OS refused to create the collector thread (pthread_create EAGAIN: RLIMIT_NPROC, a cgroup pids +// limit). Nothing else can serve the pending requests, so the mutator keeps the conn and runs the +// collection itself at its stopIfNecessary() polls, the same way it does after stealing the conn in +// requestCollection(). A stop request that releaseAccessSlow() left for the collector is withdrawn +// for the same reason: the collector that would have resumed the mutator does not exist. +void Heap::takeConnBecauseCollectorThreadCouldNotStart(const AbstractLocker&) +{ + dataLogLnIf(HeapInternal::verbose, "Taking the conn: the collector thread could not start."); + for (;;) { + unsigned oldState = m_worldState.load(); + unsigned newState = (oldState | mutatorHasConnBit) & ~stoppedBit; + if (m_worldState.compareExchangeWeak(oldState, newState)) + return; + } +} + +// Called by a mutator that waits for a collection while no collector thread can be created. With the +// conn, the mutator runs only the phases that stop the world; the concurrent phase is the collector's, +// and with the stochastic scheduler it lasts until marking terminates or the mutator allocates its +// headroom. A waiting mutator allocates nothing, so without a collector the collection would never +// end. So this thread hands the conn to the collector side and runs the collector's phases itself. +// The collector side hands the conn straight back when it needs the world stopped (stopTheMutator() +// sees that the mutator has heap access), and the caller's loop continues as the mutator. No real +// collector thread can appear in the meantime: only this thread's notifies start one. +void Heap::collectAsTheCollectorBecauseCollectorThreadCouldNotStart() +{ + dataLogLnIf(HeapInternal::verbose, "Running the collector's phases on the mutator thread: the collector thread could not start."); + for (;;) { + unsigned oldState = m_worldState.load(); + RELEASE_ASSERT(oldState & hasAccessBit); + if (!(oldState & mutatorHasConnBit)) + break; + if (m_worldState.compareExchangeWeak(oldState, oldState & ~mutatorHasConnBit)) + break; + } + sanitizeStackForVM(vm()); + // The collector side waits for the mutator to run a pending epilogue before it stops the world. + // Here both sides are this thread, so run it first. + handleNeedCollectionEpilogue(); + CollectingScope collectingScope(*this); + collectInCollectorThread(); + // Either stopTheMutator() gave the conn back, or the collection finished with the conn on the + // collector side, which is the idle state requestCollection() steals from. } void Heap::relinquishConn() @@ -2411,8 +2476,8 @@ Heap::Ticket Heap::requestCollection(GCRequest request) m_requests.append(request); m_lastGrantedTicket++; - if (!(m_worldState.load() & mutatorHasConnBit)) - m_threadCondition->notifyOne(locker); + if (!(m_worldState.load() & mutatorHasConnBit) && !m_threadCondition->notifyOne(locker)) + takeConnBecauseCollectorThreadCouldNotStart(locker); return m_lastGrantedTicket; } diff --git a/Source/JavaScriptCore/heap/Heap.h b/Source/JavaScriptCore/heap/Heap.h index fba5e652eb1a..1d1db8d0fb17 100644 --- a/Source/JavaScriptCore/heap/Heap.h +++ b/Source/JavaScriptCore/heap/Heap.h @@ -758,7 +758,9 @@ class Heap { void handleNeedCollectionEpilogue(); bool relinquishConn(unsigned); - void finishRelinquishingConn(); + bool finishRelinquishingConn(); + void takeConnBecauseCollectorThreadCouldNotStart(const AbstractLocker&); + void collectAsTheCollectorBecauseCollectorThreadCouldNotStart(); void setNeedCollectionEpilogue(); void waitWhileNeedCollectionEpilogue(); diff --git a/Source/JavaScriptCore/jit/JITWorklist.cpp b/Source/JavaScriptCore/jit/JITWorklist.cpp index 0c3b7436018d..86d4a9045e12 100644 --- a/Source/JavaScriptCore/jit/JITWorklist.cpp +++ b/Source/JavaScriptCore/jit/JITWorklist.cpp @@ -43,7 +43,9 @@ WTF_MAKE_TZONE_ALLOCATED_IMPL(JITWorklist); JITWorklist::JITWorklist() : m_lock(Box::create()) - , m_planEnqueued(AutomaticThreadCondition::create()) + // A compiler thread the OS refuses to create (pthread_create EAGAIN under a thread or pid limit) + // is not fatal: its plans stay queued and wakeThreads() / waitUntilAllPlansForVMAreReady() retry. + , m_planEnqueued(AutomaticThreadCondition::create(AutomaticThreadCondition::StartFailure::Retry)) { m_maximumNumberOfConcurrentCompilationsPerTier = { Options::numberOfBaselineCompilerThreads(), @@ -142,7 +144,11 @@ void JITWorklist::wakeThreads(const AbstractLocker& locker, unsigned enqueuedTie targetNumThreads = std::min(targetNumThreads, maxThreads); } while (m_numberOfActiveThreads < targetNumThreads) { - m_planEnqueued->notifyOne(locker); + // False means the OS refused to create a thread. The plan stays queued for the next enqueue + // (or for waitUntilAllPlansForVMAreReady()), and an active-thread count that never came to + // life must not be recorded. + if (!m_planEnqueued->notifyOne(locker)) + return; m_numberOfActiveThreads++; } ASSERT(m_numberOfActiveThreads >= 1); @@ -285,11 +291,15 @@ void JITWorklist::waitUntilAllPlansForVMAreReady(VM& vm) for (;;) { bool allAreCompiled = true; + bool hasQueuedPlan = false; for (const auto& entry : m_plans) { if (entry.value->vm() != &vm) continue; - if (entry.value->stage() != JITPlanStage::Ready) { - allAreCompiled = false; + if (entry.value->stage() == JITPlanStage::Ready) + continue; + allAreCompiled = false; + if (entry.value->stage() == JITPlanStage::Preparing) { + hasQueuedPlan = true; break; } } @@ -297,6 +307,21 @@ void JITWorklist::waitUntilAllPlansForVMAreReady(VM& vm) if (allAreCompiled) break; + if (hasQueuedPlan && !m_numberOfActiveThreads) { + // No compiler thread is awake to take the queued plans: every attempt to create one + // failed under a thread or pid limit. Try again now. If the OS still refuses, nobody + // will ever compile them, so cancel them instead of waiting forever. Their code blocks + // keep running in the lower tier and may enqueue again later. + if (m_planEnqueued->notifyOne(locker)) + m_numberOfActiveThreads++; + else { + removeMatchingPlansForVMWithLock(locker, vm, [](JITPlan& plan) { + return plan.stage() == JITPlanStage::Preparing; + }); + continue; + } + } + m_planCompiledOrCancelled.wait(*m_lock); } } @@ -444,6 +469,12 @@ template void JITWorklist::removeMatchingPlansForVM(VM& vm, const MatchFunction& matches) { Locker locker { *m_lock }; + removeMatchingPlansForVMWithLock(locker, vm, matches); +} + +template +void JITWorklist::removeMatchingPlansForVMWithLock(const AbstractLocker& locker, VM& vm, const MatchFunction& matches) +{ UncheckedKeyHashSet deadPlanKeys; for (auto& entry : m_plans) { JITPlan* plan = entry.value.get(); @@ -466,7 +497,7 @@ void JITWorklist::removeMatchingPlansForVM(VM& vm, const MatchFunction& matches) } queue.swap(newQueue); } - ASSERT(!m_totalLoad == (!queueLength(locker) && !totalOngoingCompilations(locker))); + ASSERT_UNUSED(locker, !m_totalLoad == (!queueLength(locker) && !totalOngoingCompilations(locker))); bool didCancelPlans = !deadPlanKeys.isEmpty(); for (JITCompilationKey key : deadPlanKeys) diff --git a/Source/JavaScriptCore/jit/JITWorklist.h b/Source/JavaScriptCore/jit/JITWorklist.h index bc02159b6bb6..191583edfb52 100644 --- a/Source/JavaScriptCore/jit/JITWorklist.h +++ b/Source/JavaScriptCore/jit/JITWorklist.h @@ -100,6 +100,8 @@ class JITWorklist { template void removeMatchingPlansForVM(VM&, const MatchFunction&); + template + void removeMatchingPlansForVMWithLock(const AbstractLocker&, VM&, const MatchFunction&); State removeAllReadyPlansForVM(VM&, Vector, 8>&, JITCompilationKey); diff --git a/Source/WTF/wtf/AutomaticThread.cpp b/Source/WTF/wtf/AutomaticThread.cpp index b683935d0713..2951ccb5ac19 100644 --- a/Source/WTF/wtf/AutomaticThread.cpp +++ b/Source/WTF/wtf/AutomaticThread.cpp @@ -39,42 +39,53 @@ static constexpr bool verbose = false; Ref AutomaticThreadCondition::create() { - return adoptRef(*new AutomaticThreadCondition); + return create(StartFailure::Crash); } -AutomaticThreadCondition::AutomaticThreadCondition() = default; +Ref AutomaticThreadCondition::create(StartFailure startFailure) +{ + return adoptRef(*new AutomaticThreadCondition(startFailure)); +} + +AutomaticThreadCondition::AutomaticThreadCondition(StartFailure startFailure) + : m_startFailure(startFailure) +{ +} AutomaticThreadCondition::~AutomaticThreadCondition() = default; -void AutomaticThreadCondition::notifyOne(const AbstractLocker& locker) +bool AutomaticThreadCondition::notifyOne(const AbstractLocker& locker) { for (auto& thread : m_threads) { if (thread->isWaiting(locker)) { thread->notify(locker); - return; + return true; } } for (auto& thread : m_threads) { if (!thread->hasUnderlyingThread(locker)) { - thread->start(locker); - return; + // If this one fails, the others would fail for the same reason: do not try them. + return thread->start(locker); } } m_condition.notifyOne(); + return true; } -void AutomaticThreadCondition::notifyAll(const AbstractLocker& locker) +bool AutomaticThreadCondition::notifyAll(const AbstractLocker& locker) { m_condition.notifyAll(); + bool startedAll = true; for (auto& thread : m_threads) { if (thread->isWaiting(locker)) thread->notify(locker); - else if (!thread->hasUnderlyingThread(locker)) - thread->start(locker); + else if (startedAll && !thread->hasUnderlyingThread(locker)) + startedAll = thread->start(locker); } + return startedAll; } void AutomaticThreadCondition::wait(Lock& lock) @@ -160,7 +171,7 @@ void AutomaticThread::join() m_isRunningCondition.wait(*m_lock); } -void AutomaticThread::start(const AbstractLocker&) +bool AutomaticThread::start(const AbstractLocker&) { RELEASE_ASSERT(m_isRunning); @@ -180,7 +191,7 @@ void AutomaticThread::start(const AbstractLocker&) break; } - Thread::create( + RefPtr thread = Thread::tryCreate( name(), [=, this] () { if (verbose) @@ -245,7 +256,19 @@ void AutomaticThread::start(const AbstractLocker&) } RELEASE_ASSERT(result == WorkResult::Continue); } - }, m_threadType, Thread::defaultQOS, Thread::defaultSchedulingPolicy, stackSpec)->detach(); + }, m_threadType, Thread::defaultQOS, Thread::defaultSchedulingPolicy, stackSpec); + + if (!thread) { + // The entry point (and its ref to this) was destroyed without running. + m_hasUnderlyingThread = false; + RELEASE_ASSERT_WITH_MESSAGE(m_condition->startFailure() == AutomaticThreadCondition::StartFailure::Retry, "Could not create the underlying thread for %s", name().characters()); + if (verbose) + dataLog(RawPointer(this), ": Could not create the underlying thread; will retry on the next notify.\n"); + return false; + } + + thread->detach(); + return true; } void AutomaticThread::threadDidStart() diff --git a/Source/WTF/wtf/AutomaticThread.h b/Source/WTF/wtf/AutomaticThread.h index 447e6cbece5d..ce2a43e0b369 100644 --- a/Source/WTF/wtf/AutomaticThread.h +++ b/Source/WTF/wtf/AutomaticThread.h @@ -73,12 +73,29 @@ class AutomaticThread; class AutomaticThreadCondition : public ThreadSafeRefCounted { public: + // What notifyOne()/notifyAll() do when an AutomaticThread has no underlying thread and the OS + // refuses to create one (EAGAIN from pthread_create: RLIMIT_NPROC, a cgroup pids limit). + enum class StartFailure : uint8_t { + // Crash. This is the default: the work would otherwise wait for a thread that never comes. + Crash, + // Leave the AutomaticThread without an underlying thread and return false from the notify. + // The next notify tries again. Only for users that act on a false return, so that the work + // still completes without the thread: ParallelHelperPool (the client runs the task itself), + // JSC::Heap (the mutator collects) and JSC::JITWorklist (plans stay queued or are cancelled). + Retry, + }; + static WTF_EXPORT_PRIVATE Ref NODELETE create(); + static WTF_EXPORT_PRIVATE Ref NODELETE create(StartFailure); WTF_EXPORT_PRIVATE ~AutomaticThreadCondition(); - WTF_EXPORT_PRIVATE void notifyOne(const AbstractLocker&); - WTF_EXPORT_PRIVATE void notifyAll(const AbstractLocker&); + // Both return false only when a thread had to be started for this notification and the OS + // refused, which can only happen with StartFailure::Retry. + WTF_EXPORT_PRIVATE bool notifyOne(const AbstractLocker&); + WTF_EXPORT_PRIVATE bool notifyAll(const AbstractLocker&); + + StartFailure startFailure() const { return m_startFailure; } // You can reuse this condition for other things, just as you would any other condition. // However, since conflating conditions could lead to thundering herd, it's best to avoid it. @@ -91,7 +108,7 @@ class AutomaticThreadCondition : public ThreadSafeRefCounted> m_threads; + const StartFailure m_startFailure; }; class WTF_EXPORT_PRIVATE AutomaticThread : public ThreadSafeRefCounted, public CanMakeThreadSafeCheckedPtr { @@ -191,7 +209,9 @@ class WTF_EXPORT_PRIVATE AutomaticThread : public ThreadSafeRefCounted m_lock; diff --git a/Source/WTF/wtf/ParallelHelperPool.cpp b/Source/WTF/wtf/ParallelHelperPool.cpp index 4e35368d1fbd..ef54d62ab863 100644 --- a/Source/WTF/wtf/ParallelHelperPool.cpp +++ b/Source/WTF/wtf/ParallelHelperPool.cpp @@ -131,7 +131,9 @@ Ref ParallelHelperPool::create(ASCIILiteral threadName) ParallelHelperPool::ParallelHelperPool(ASCIILiteral threadName) : m_lock(Box::create()) - , m_workAvailableCondition(AutomaticThreadCondition::create()) + // A helper the OS refuses to create (pthread_create EAGAIN under a thread or pid limit) is not + // fatal: the client runs the task on its own thread and the helpers retry on the next task. + , m_workAvailableCondition(AutomaticThreadCondition::create(AutomaticThreadCondition::StartFailure::Retry)) , m_threadName(threadName) { } @@ -144,6 +146,9 @@ ParallelHelperPool::~ParallelHelperPool() Locker locker { *m_lock }; m_isDying = true; m_workAvailableCondition->notifyAll(locker); + // A helper that never got an underlying thread has nothing to join. + for (RefPtr& thread : m_threads) + thread->tryStop(locker); } for (RefPtr& thread : m_threads) diff --git a/Source/WTF/wtf/Threading.cpp b/Source/WTF/wtf/Threading.cpp index f062105de8ad..91cb490e7d65 100644 --- a/Source/WTF/wtf/Threading.cpp +++ b/Source/WTF/wtf/Threading.cpp @@ -312,6 +312,13 @@ void Thread::entryPoint(NewThreadContext* newThreadContext) } Ref Thread::create(ASCIILiteral name, Function&& entryPoint, ThreadType threadType, QOS qos, SchedulingPolicy schedulingPolicy, StackAllocationSpecification stackSpec) +{ + RefPtr thread = tryCreate(name, WTF::move(entryPoint), threadType, qos, schedulingPolicy, stackSpec); + RELEASE_ASSERT(thread); + return thread.releaseNonNull(); +} + +RefPtr Thread::tryCreate(ASCIILiteral name, Function&& entryPoint, ThreadType threadType, QOS qos, SchedulingPolicy schedulingPolicy, StackAllocationSpecification stackSpec) { WTF::initialize(); @@ -326,8 +333,11 @@ Ref Thread::create(ASCIILiteral name, Function&& entryPoint, Thr if (maybeSize) stackSpec = StackAllocationSpecification::RequestSize(maybeSize.value()); } - bool success = thread->establishHandle(context.get(), stackSpec, qos, schedulingPolicy); - RELEASE_ASSERT(success); + if (!thread->establishHandle(context.get(), stackSpec, qos, schedulingPolicy)) { + // Thread::entryPoint never runs, so take back the ref it would have adopted. + context->deref(); + return nullptr; + } #if HAVE(STACK_BOUNDS_FOR_NEW_THREAD) thread->m_stack = StackBounds::newThreadStackBounds(thread->m_handle); diff --git a/Source/WTF/wtf/Threading.h b/Source/WTF/wtf/Threading.h index 319a94f31cbb..54b9d8c2954f 100644 --- a/Source/WTF/wtf/Threading.h +++ b/Source/WTF/wtf/Threading.h @@ -136,10 +136,14 @@ class WTF_CAPABILITY("is current") Thread : public ThreadSafeRefCountedAndCanMak static dispatch_qos_class_t dispatchQOSClass(QOS); #endif - // Returns nullptr if thread creation failed. + // Crashes 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 Ref create(ASCIILiteral threadName, Function&&, ThreadType = ThreadType::Unknown, QOS = defaultQOS, SchedulingPolicy = defaultSchedulingPolicy, StackAllocationSpecification = { }); + // Returns nullptr if the OS refused to create the thread (EAGAIN from pthread_create: RLIMIT_NPROC, + // a cgroup pids limit, or a system-wide thread limit). The entry point is destroyed without running. + WTF_EXPORT_PRIVATE static RefPtr tryCreate(ASCIILiteral threadName, Function&&, ThreadType = ThreadType::Unknown, QOS = defaultQOS, SchedulingPolicy = defaultSchedulingPolicy, StackAllocationSpecification = { }); + // Returns Thread object. static Thread& currentSingleton(); @@ -395,7 +399,8 @@ class WTF_CAPABILITY("is current") Thread : public ThreadSafeRefCountedAndCanMak WordLock m_mutex; StackBounds m_stack { StackBounds::emptyBounds() }; ThreadSafeWeakHashSet m_threadGroups; - PlatformThreadHandle m_handle; + // Value-initialized so that a Thread whose establishHandle() failed destructs cleanly. + PlatformThreadHandle m_handle { }; const uint32_t m_uid; #if OS(WINDOWS) ThreadIdentifier m_id { 0 }; diff --git a/Source/WTF/wtf/win/ThreadingWin.cpp b/Source/WTF/wtf/win/ThreadingWin.cpp index 03d529edecf2..dc7facd1b508 100644 --- a/Source/WTF/wtf/win/ThreadingWin.cpp +++ b/Source/WTF/wtf/win/ThreadingWin.cpp @@ -117,7 +117,7 @@ Thread::~Thread() { // It is OK because FLSAlloc's callback will be called even before there are some open handles. // This easily ensures that all the thread resources are automatically closed. - if (m_handle != INVALID_HANDLE_VALUE) + if (m_handle && m_handle != INVALID_HANDLE_VALUE) CloseHandle(m_handle); }