Survive a refused thread creation in the GC helper pool, the collector thread, and the JIT worklist

Source/JavaScriptCore/heap/Heap.cpp

@@ -360,7 +360,10 @@ Heap::Heap(VM& vm, HeapType heapType)
     , m_sharedMutatorMarkStack(makeUnique<MarkStackArray>())
     , m_helperClient(&heapHelperPool())
     , m_threadLock(Box<Lock>::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<typename Func>
 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;
 }
 

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();

Source/JavaScriptCore/jit/JITWorklist.cpp

@@ -43,7 +43,9 @@ WTF_MAKE_TZONE_ALLOCATED_IMPL(JITWorklist);
 
 JITWorklist::JITWorklist()
     : m_lock(Box<Lock>::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,18 +291,37 @@ 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;
             }
         }
 
         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<typename MatchFunction>
 void JITWorklist::removeMatchingPlansForVM(VM& vm, const MatchFunction& matches)
 {
     Locker locker { *m_lock };
+    removeMatchingPlansForVMWithLock(locker, vm, matches);
+}
+
+template<typename MatchFunction>
+void JITWorklist::removeMatchingPlansForVMWithLock(const AbstractLocker& locker, VM& vm, const MatchFunction& matches)
+{
     UncheckedKeyHashSet<JITCompilationKey> 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)

Source/JavaScriptCore/jit/JITWorklist.h

@@ -100,6 +100,8 @@ class JITWorklist {
 
     template<typename MatchFunction>
     void removeMatchingPlansForVM(VM&, const MatchFunction&);
+    template<typename MatchFunction>
+    void removeMatchingPlansForVMWithLock(const AbstractLocker&, VM&, const MatchFunction&);
 
     State removeAllReadyPlansForVM(VM&, Vector<Ref<JITPlan>, 8>&, JITCompilationKey);
 

Source/WTF/wtf/AutomaticThread.cpp

@@ -39,42 +39,53 @@ static constexpr bool verbose = false;
 
 Ref<AutomaticThreadCondition> AutomaticThreadCondition::create()
 {
-    return adoptRef(*new AutomaticThreadCondition);
+    return create(StartFailure::Crash);
 }
 
-AutomaticThreadCondition::AutomaticThreadCondition() = default;
+Ref<AutomaticThreadCondition> 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 = 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()

Source/WTF/wtf/AutomaticThread.h

@@ -73,12 +73,29 @@ class AutomaticThread;
 
 class AutomaticThreadCondition : public ThreadSafeRefCounted<AutomaticThreadCondition> {
 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<AutomaticThreadCondition> NODELETE create();
+    static WTF_EXPORT_PRIVATE Ref<AutomaticThreadCondition> 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,14 +108,15 @@ class AutomaticThreadCondition : public ThreadSafeRefCounted<AutomaticThreadCond
 private:
     friend class AutomaticThread;
 
-    WTF_EXPORT_PRIVATE AutomaticThreadCondition();
+    WTF_EXPORT_PRIVATE AutomaticThreadCondition(StartFailure);
 
     void add(const AbstractLocker&, AutomaticThread*);
     void remove(const AbstractLocker&, AutomaticThread*);
     bool contains(const AbstractLocker&, AutomaticThread*);
 
     Condition m_condition;
     Vector<CheckedPtr<AutomaticThread>> m_threads;
+    const StartFailure m_startFailure;
 };
 
 class WTF_EXPORT_PRIVATE AutomaticThread : public ThreadSafeRefCounted<AutomaticThread>, public CanMakeThreadSafeCheckedPtr<AutomaticThread> {
@@ -191,7 +209,9 @@ class WTF_EXPORT_PRIVATE AutomaticThread : public ThreadSafeRefCounted<Automatic
 private:
     friend class AutomaticThreadCondition;
 
-    void start(const AbstractLocker&);
+    // Returns false when the OS refused to create the thread and m_condition's StartFailure is
+    // Retry. With StartFailure::Crash it crashes instead.
+    bool start(const AbstractLocker&);
 
 protected:
     Box<Lock> m_lock;