diff --git a/Source/JavaScriptCore/runtime/AtomicsObject.cpp b/Source/JavaScriptCore/runtime/AtomicsObject.cpp index 9f8833c13f67..d1fbada89a6f 100644 --- a/Source/JavaScriptCore/runtime/AtomicsObject.cpp +++ b/Source/JavaScriptCore/runtime/AtomicsObject.cpp @@ -470,7 +470,7 @@ JSValue atomicsWaitImpl(JSGlobalObject* globalObject, JSArrayType* typedArray, u case WaiterListManager::WaitSyncResult::TimedOut: return vm.smallStrings.timedOutString(); case WaiterListManager::WaitSyncResult::Terminated: - vm.throwTerminationException(); + ASSERT(vm.hasPendingTerminationException()); return { }; } RELEASE_ASSERT_NOT_REACHED(); diff --git a/Source/JavaScriptCore/runtime/VMTraps.cpp b/Source/JavaScriptCore/runtime/VMTraps.cpp index 7dd488259b83..80eeab351f3a 100644 --- a/Source/JavaScriptCore/runtime/VMTraps.cpp +++ b/Source/JavaScriptCore/runtime/VMTraps.cpp @@ -327,7 +327,7 @@ class VMTraps::SignalSender final : public ThreadSafeRefCountedcondition().notifyOne(); + vm.syncWaiter()->notifyOfTerminationRequest(); { Locker locker { *m_lock }; @@ -398,7 +398,6 @@ CONCURRENT_SAFE void VMTraps::requestThreadStopIfNeeded(Locker& locker) ASSERT(!m_threadStopRequested); ASSERT(!m_isShuttingDown); - VM& vm = this->vm(); m_stack.requestStop(); m_needToInvalidateCodeBlocks = true; @@ -409,19 +408,21 @@ CONCURRENT_SAFE void VMTraps::requestThreadStopIfNeeded(Locker& locker) // has received the trap request. We'll call it from another thread so that // requestThreadStopIfNeeded() does not block. if (!m_signalSender) - m_signalSender = adoptRef(new SignalSender(locker, vm)); + m_signalSender = adoptRef(new SignalSender(locker, vm())); m_signalSender->notify(locker); } #else UNUSED_PARAM(locker); #endif - if (hasTrapBit(NeedTermination)) - vm.syncWaiter()->condition().notifyOne(); - m_threadStopRequested = true; } +CONCURRENT_SAFE void VMTraps::notifySyncWaiterOfTermination() +{ + vm().syncWaiter()->notifyOfTerminationRequest(); +} + CONCURRENT_SAFE void VMTraps::updateThreadStopRequestIfNeeded() { Locker locker { *m_trapSignalingLock }; diff --git a/Source/JavaScriptCore/runtime/VMTraps.h b/Source/JavaScriptCore/runtime/VMTraps.h index d84d871e3185..96029b867be3 100644 --- a/Source/JavaScriptCore/runtime/VMTraps.h +++ b/Source/JavaScriptCore/runtime/VMTraps.h @@ -249,8 +249,13 @@ class VMTraps { // Trap bit must be set before we update the thread stop request. if (isAsyncEvent(event)) updateThreadStopRequestIfNeeded(); + // A thread parked in Atomics.wait / memory.atomic.wait handles no traps; wake it so it sees this one. + if (event == NeedTermination) + notifySyncWaiterOfTermination(); } + JS_EXPORT_PRIVATE CONCURRENT_SAFE void notifySyncWaiterOfTermination(); + // The following returns true if a trap was handled. bool handleTraps(BitField mask = AsyncEvents); bool handleTrapsIfNeeded(BitField mask = AsyncEvents); diff --git a/Source/JavaScriptCore/runtime/WaiterListManager.cpp b/Source/JavaScriptCore/runtime/WaiterListManager.cpp index 5df3695bcf02..88f4186bbc14 100644 --- a/Source/JavaScriptCore/runtime/WaiterListManager.cpp +++ b/Source/JavaScriptCore/runtime/WaiterListManager.cpp @@ -62,6 +62,44 @@ Waiter::Waiter(JSPromise* promise) { } +Waiter::~Waiter() = default; + +void Waiter::setParkedList(RefPtr&& list) +{ + ASSERT(!m_isAsync); + Locker locker { m_parkedListLock }; + m_parkedList = WTF::move(list); +} + +void Waiter::notifyOfTerminationRequest() +{ + ASSERT(!m_isAsync); + RefPtr parkedList; + { + Locker locker { m_parkedListLock }; + parkedList = m_parkedList; + } + // Not parked (yet): waitSyncImpl publishes the list before it tests for the request, so a wait + // that starts after this sees the NeedTermination trap our caller has already fired. + if (!parkedList) + return; + // Notify under the list's lock, which the waiter holds from testing for the request until it is + // parked, so that the notification cannot fall in between. + Locker listLocker { parkedList->lock }; + m_condition.notifyOne(); +} + +// A termination is only ever established (VM::hasTerminationRequest) by the mutator thread itself, +// which cannot do that while it is parked in a wait. Another thread asking this VM to terminate +// (VM::notifyNeedTermination) is visible as the pending NeedTermination trap. Neither interrupts the +// wait while termination is being deferred (DeferTermination); it takes effect when the deferral ends. +static ALWAYS_INLINE bool shouldStopWaitingForTermination(VM& vm) +{ + if (vm.traps().isDeferringTermination()) [[unlikely]] + return false; + return vm.hasTerminationRequest() || vm.traps().needHandling(VMTraps::NeedTermination); +} + WaiterListManager& WaiterListManager::singleton() { @@ -91,10 +129,13 @@ WaiterListManager::WaitSyncResult WaiterListManager::waitSyncImpl(VM& vm, ValueT list->addLast(listLocker, syncWaiter); dataLogLnIf(WaiterListsManagerInternal::verbose, " added a new SyncWaiter=", syncWaiter.get(), " to a waiterList for ptr ", RawPointer(ptr)); + syncWaiter->setParkedList(list.copyRef()); - while (syncWaiter->isOnList() && time.now() < time && !vm.hasTerminationRequest()) + while (syncWaiter->isOnList() && time.now() < time && !shouldStopWaitingForTermination(vm)) syncWaiter->condition().waitUntil(list->lock, time.approximate()); + syncWaiter->setParkedList(nullptr); + // At this point, syncWaiter should be either notified (dequeued) or timeout (not dequeued). bool didGetDequeued = !syncWaiter->isOnList(); if (didGetDequeued) @@ -102,8 +143,20 @@ WaiterListManager::WaitSyncResult WaiterListManager::waitSyncImpl(VM& vm, ValueT didGetDequeued = list->findAndRemove(listLocker, syncWaiter); ASSERT(didGetDequeued); - return vm.hasTerminationRequest() ? WaitSyncResult::Terminated : WaitSyncResult::TimedOut; + if (!shouldStopWaitingForTermination(vm)) + return WaitSyncResult::TimedOut; } + + // The wait was cut short by a termination request: leave with it established and the + // TerminationException thrown, consuming the trap if another thread's request is what woke us + // (as VMTraps::handleTraps() would, minus jettisoning the code blocks that have trap breakpoints + // installed, which throwing from here does not need). + ASSERT(!vm.traps().isDeferringTermination()); + if (vm.traps().clearTrap(VMTraps::NeedTermination)) + vm.setHasTerminationRequest(); + if (!vm.hasPendingTerminationException()) + vm.throwTerminationException(); + return WaitSyncResult::Terminated; } template diff --git a/Source/JavaScriptCore/runtime/WaiterListManager.h b/Source/JavaScriptCore/runtime/WaiterListManager.h index 1d829772d213..4c239885df37 100644 --- a/Source/JavaScriptCore/runtime/WaiterListManager.h +++ b/Source/JavaScriptCore/runtime/WaiterListManager.h @@ -37,12 +37,21 @@ namespace JSC { enum class AtomicsWaitType : uint8_t { Sync, Async }; enum class AtomicsWaitValidation : uint8_t { Pass, Fail }; +class WaiterList; + class Waiter final : public WTF::BasicRawSentinelNode, public ThreadSafeRefCounted { WTF_MAKE_TZONE_ALLOCATED(Waiter); public: Waiter(VM*); Waiter(JSPromise*); + ~Waiter(); + + // Sync waiter only; any thread. Wake the VM's thread if it is parked in Atomics.wait / + // memory.atomic.wait so that it sees the termination request that was just fired. + void notifyOfTerminationRequest(); + // Sync waiter only; its own thread, holding the lock of the list it is (un)parking on. + void setParkedList(RefPtr&&); bool isAsync() const { @@ -117,6 +126,9 @@ class Waiter final : public WTF::BasicRawSentinelNode, public ThreadSafe ThreadSafeWeakPtr m_ticket { nullptr }; RefPtr m_timer { nullptr }; Condition m_condition; + // Sync waiter only: the list it is parked on, for notifyOfTerminationRequest() to lock. + Lock m_parkedListLock; + RefPtr m_parkedList WTF_GUARDED_BY_LOCK(m_parkedListLock); bool m_isAsync { false }; }; diff --git a/Source/JavaScriptCore/wasm/WasmOperationsInlines.h b/Source/JavaScriptCore/wasm/WasmOperationsInlines.h index 544bfc750544..af6b7415745a 100644 --- a/Source/JavaScriptCore/wasm/WasmOperationsInlines.h +++ b/Source/JavaScriptCore/wasm/WasmOperationsInlines.h @@ -747,7 +747,7 @@ static inline int32_t waitImpl(VM& vm, ValueType* pointer, ValueType expectedVal case WaiterListManager::WaitSyncResult::TimedOut: return static_cast(result); case WaiterListManager::WaitSyncResult::Terminated: - vm.throwTerminationException(); + ASSERT(vm.hasPendingTerminationException()); return -1; } RELEASE_ASSERT_NOT_REACHED();