From da28e0137fb5f6b8d7b951d4a3fa02e4f2be07be Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 22 Aug 2026 14:30:05 +0000 Subject: [PATCH] Survive a refused thread creation in the GC helper pool, the collector thread, and the JIT worklist WTF::Thread::create crashes when pthread_create fails. Under a thread or pid limit (RLIMIT_NPROC, a cgroup pids.max full of the process's own children) the first full GC after the limit is reached aborts the process from AutomaticThread::start, because the heap helper threads and the collector thread are AutomaticThreads that exit after 10 seconds of idle time and are created again on demand. Add Thread::tryCreate, which returns nullptr instead of crashing, and let AutomaticThreadCondition choose what a failed start means. The default (StartFailure::Crash) keeps the old behavior. With StartFailure::Retry the AutomaticThread is left without an underlying thread, the notify returns false, and the next notify tries again. ParallelHelperPool uses Retry: the client runs the task on its own thread. Its destructor stops threads that never started before it joins them. JSC::Heap uses Retry. When the collector thread cannot start, the mutator takes the conn back and runs the collection itself. A mutator that waits for a collection also runs the collector's phases, because the concurrent phase is the collector's and, with the stochastic scheduler, a waiting mutator would otherwise never see it end. JITWorklist uses Retry. wakeThreads counts only threads that were notified or started. waitUntilAllPlansForVMAreReady cancels plans that no compiler thread can take, so deleteAllCode does not wait forever. The code blocks stay in the lower tier and can enqueue again. --- Source/JavaScriptCore/heap/Heap.cpp | 83 ++++++++++++++++++++--- Source/JavaScriptCore/heap/Heap.h | 4 +- Source/JavaScriptCore/jit/JITWorklist.cpp | 41 +++++++++-- Source/JavaScriptCore/jit/JITWorklist.h | 2 + Source/WTF/wtf/AutomaticThread.cpp | 47 +++++++++---- Source/WTF/wtf/AutomaticThread.h | 28 ++++++-- Source/WTF/wtf/ParallelHelperPool.cpp | 7 +- Source/WTF/wtf/Threading.cpp | 14 +++- Source/WTF/wtf/Threading.h | 9 ++- Source/WTF/wtf/win/ThreadingWin.cpp | 2 +- 10 files changed, 200 insertions(+), 37 deletions(-) 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); }