From cf1a620331e9dcc35022d2980da5f87e3e3a274c Mon Sep 17 00:00:00 2001 From: James Hugard Date: Thu, 30 Jul 2026 10:15:59 -0700 Subject: [PATCH 1/3] Fix WaitTimer teardown and wrapper lifetime races (#1005) * Add deterministic STL WaitTimer teardown repro * Fix STL WaitTimer queue teardown lifetime * Add WaitTimer wrapper termination repro * Serialize WaitTimer wrapper termination (cherry picked from commit 0650991cdbd845db9a64d9f0a82177487438d18b) --- Build/libHttpClient.Linux/CMakeLists.txt | 169 ++++++++- Source/Task/WaitTimer.h | 15 + Source/Task/WaitTimer_stl.cpp | 328 +++++++++++++++--- Source/Task/WaitTimer_win32.cpp | 19 +- Source/Task/iOS/ios_WaitTimer.mm | 19 +- .../SingleQueuePollStrandRepro.cpp | 0 .../TaskQueueStarvationRepro.cpp | 0 .../WaitTimerQueueRetirementRepro.cpp | 127 +++++++ .../WaitTimerQueueTeardownRepro.cpp | 147 ++++++++ Tests/StandaloneTests/WaitTimerTestMemory.cpp | 25 ++ .../WaitTimerWrapperTerminateRepro.cpp | 161 +++++++++ 11 files changed, 961 insertions(+), 49 deletions(-) rename Tests/{TaskQueueStarvation => StandaloneTests}/SingleQueuePollStrandRepro.cpp (100%) rename Tests/{TaskQueueStarvation => StandaloneTests}/TaskQueueStarvationRepro.cpp (100%) create mode 100644 Tests/StandaloneTests/WaitTimerQueueRetirementRepro.cpp create mode 100644 Tests/StandaloneTests/WaitTimerQueueTeardownRepro.cpp create mode 100644 Tests/StandaloneTests/WaitTimerTestMemory.cpp create mode 100644 Tests/StandaloneTests/WaitTimerWrapperTerminateRepro.cpp diff --git a/Build/libHttpClient.Linux/CMakeLists.txt b/Build/libHttpClient.Linux/CMakeLists.txt index 15105c71..93e4b797 100644 --- a/Build/libHttpClient.Linux/CMakeLists.txt +++ b/Build/libHttpClient.Linux/CMakeLists.txt @@ -273,7 +273,7 @@ find_package(Threads REQUIRED) add_executable( "TaskQueueStarvationTests.Linux" - "${PATH_TO_ROOT}/Tests/TaskQueueStarvation/TaskQueueStarvationRepro.cpp" + "${PATH_TO_ROOT}/Tests/StandaloneTests/TaskQueueStarvationRepro.cpp" ) target_include_directories( @@ -330,7 +330,7 @@ set_tests_properties( # the libHttpClient.Linux target. add_executable( "SingleQueuePollStrandTests.Linux" - "${PATH_TO_ROOT}/Tests/TaskQueueStarvation/SingleQueuePollStrandRepro.cpp" + "${PATH_TO_ROOT}/Tests/StandaloneTests/SingleQueuePollStrandRepro.cpp" ) target_include_directories( @@ -376,4 +376,169 @@ set_tests_properties( LABELS "taskqueue" ) +################################################# +### WaitTimer queue-teardown race repro ### +################################################# +# This white-box repro links the normal library for its dependencies but +# compiles the private STL timer implementation into the executable so it can +# deterministically pause the worker after Pop(). The fix makes dispatch state +# independent from WaitTimerImpl lifetime, so a paused worker can no longer +# observe a destroyed timer. +add_executable( + "WaitTimerQueueTeardownTests.Linux" + "${PATH_TO_ROOT}/Tests/StandaloneTests/WaitTimerQueueTeardownRepro.cpp" + "${PATH_TO_ROOT}/Tests/StandaloneTests/WaitTimerTestMemory.cpp" + "${PATH_TO_ROOT}/Source/Task/WaitTimer_stl.cpp" +) + +target_include_directories( + "WaitTimerQueueTeardownTests.Linux" + PRIVATE + "${COMMON_INCLUDE_DIRS}" + "${LINUX_INCLUDE_DIRS}" +) + +target_link_libraries( + "WaitTimerQueueTeardownTests.Linux" + PRIVATE + "${PROJECT_NAME}" + Threads::Threads + ${CMAKE_DL_LIBS} +) + +if (NOT BUILD_SHARED_LIBS) + target_link_libraries( + "WaitTimerQueueTeardownTests.Linux" + PRIVATE + "${LIBCURL_BINARY_PATH}" + "${LIBSSL_BINARY_PATH}" + "${LIBCRYPTO_BINARY_PATH}" + ) +endif() + +target_set_flags( + "WaitTimerQueueTeardownTests.Linux" + "${FLAGS}" + "${FLAGS_DEBUG}" + "${FLAGS_RELEASE}" +) + +add_test( + NAME waittimer-queue-teardown-linux + COMMAND "WaitTimerQueueTeardownTests.Linux" +) +set_tests_properties( + waittimer-queue-teardown-linux + PROPERTIES + LABELS "taskqueue" +) + +################################################# +### WaitTimer queue-retirement race repro ### +################################################# +# White-box regression for the shared TimerQueue's last-owner retirement. It +# compiles the private STL timer implementation so it can pause inside +# RemoveTimer while the retiring owner holds g_timerQueueMutex, proving a +# concurrent Initialize cannot adopt a queue that is being retired. +add_executable( + "WaitTimerQueueRetirementTests.Linux" + "${PATH_TO_ROOT}/Tests/StandaloneTests/WaitTimerQueueRetirementRepro.cpp" + "${PATH_TO_ROOT}/Tests/StandaloneTests/WaitTimerTestMemory.cpp" + "${PATH_TO_ROOT}/Source/Task/WaitTimer_stl.cpp" +) + +target_include_directories( + "WaitTimerQueueRetirementTests.Linux" + PRIVATE + "${COMMON_INCLUDE_DIRS}" + "${LINUX_INCLUDE_DIRS}" +) + +target_link_libraries( + "WaitTimerQueueRetirementTests.Linux" + PRIVATE + "${PROJECT_NAME}" + Threads::Threads + ${CMAKE_DL_LIBS} +) + +if (NOT BUILD_SHARED_LIBS) + target_link_libraries( + "WaitTimerQueueRetirementTests.Linux" + PRIVATE + "${LIBCURL_BINARY_PATH}" + "${LIBSSL_BINARY_PATH}" + "${LIBCRYPTO_BINARY_PATH}" + ) +endif() + +target_set_flags( + "WaitTimerQueueRetirementTests.Linux" + "${FLAGS}" + "${FLAGS_DEBUG}" + "${FLAGS_RELEASE}" +) + +add_test( + NAME waittimer-queue-retirement-linux + COMMAND "WaitTimerQueueRetirementTests.Linux" +) +set_tests_properties( + waittimer-queue-retirement-linux + PROPERTIES + LABELS "taskqueue" +) + +################################################# +### WaitTimer wrapper lifetime race repro ### +################################################# +add_executable( + "WaitTimerWrapperTerminateTests.Linux" + "${PATH_TO_ROOT}/Tests/StandaloneTests/WaitTimerWrapperTerminateRepro.cpp" + "${PATH_TO_ROOT}/Tests/StandaloneTests/WaitTimerTestMemory.cpp" + "${PATH_TO_ROOT}/Source/Task/WaitTimer_stl.cpp" +) + +target_include_directories( + "WaitTimerWrapperTerminateTests.Linux" + PRIVATE + "${COMMON_INCLUDE_DIRS}" + "${LINUX_INCLUDE_DIRS}" +) + +target_link_libraries( + "WaitTimerWrapperTerminateTests.Linux" + PRIVATE + "${PROJECT_NAME}" + Threads::Threads + ${CMAKE_DL_LIBS} +) + +if (NOT BUILD_SHARED_LIBS) + target_link_libraries( + "WaitTimerWrapperTerminateTests.Linux" + PRIVATE + "${LIBCURL_BINARY_PATH}" + "${LIBSSL_BINARY_PATH}" + "${LIBCRYPTO_BINARY_PATH}" + ) +endif() + +target_set_flags( + "WaitTimerWrapperTerminateTests.Linux" + "${FLAGS}" + "${FLAGS_DEBUG}" + "${FLAGS_RELEASE}" +) + +add_test( + NAME waittimer-wrapper-terminate-linux + COMMAND "WaitTimerWrapperTerminateTests.Linux" +) +set_tests_properties( + waittimer-wrapper-terminate-linux + PROPERTIES + LABELS "taskqueue" +) + export(TARGETS ${PROJECT_NAME} FILE ${PROJECT_NAME}Config.cmake) diff --git a/Source/Task/WaitTimer.h b/Source/Task/WaitTimer.h index 0a80af5a..342a0880 100644 --- a/Source/Task/WaitTimer.h +++ b/Source/Task/WaitTimer.h @@ -1,11 +1,25 @@ #pragma once +#include + namespace OS { using WaitTimerCallback = void(_In_opt_ void*); class WaitTimerImpl; + // Source-internal test seam for deterministic STL WaitTimer lifetime + // regressions. It is not exported as part of the public libHttpClient API. + struct WaitTimerTestHooks + { + virtual void WaitTimerImplDestructionStarted() noexcept {} + virtual bool BeforeTimerInvoke() noexcept { return true; } + virtual void BeforeTimerQueueRetirement() noexcept {} + virtual bool WaitTimerOperationLoaded(bool) noexcept { return true; } + }; + + void WaitTimerSetTestHooks(_In_opt_ WaitTimerTestHooks* hooks) noexcept; + // A wait timer holds a single timeout expressed as a monotonic due time. // Calling Start will reset any pending timeout. class WaitTimer @@ -29,6 +43,7 @@ namespace OS uint64_t GetDueTime(_In_ uint32_t msFromNow) noexcept; private: + DefaultUnnamedMutex m_mutex; std::atomic m_impl; }; } diff --git a/Source/Task/WaitTimer_stl.cpp b/Source/Task/WaitTimer_stl.cpp index e4b61d5d..be4f8940 100644 --- a/Source/Task/WaitTimer_stl.cpp +++ b/Source/Task/WaitTimer_stl.cpp @@ -30,6 +30,65 @@ namespace OS { class TimerQueue; + class WaitTimerState + { + public: + WaitTimerState(_In_opt_ void* context, _In_ WaitTimerCallback* callback) noexcept + : m_context(context), m_callback(callback) + {} + + void BeginTerminate() noexcept + { + m_terminating.store(true, std::memory_order_release); + } + + bool TryBeginDispatch() noexcept + { + if (m_terminating.load(std::memory_order_acquire)) + { + return false; + } + + std::lock_guard lock{ m_mutex }; + if (m_terminating.load(std::memory_order_relaxed)) + { + return false; + } + + ++m_inFlightDispatch; + return true; + } + + void EndDispatch() noexcept + { + std::lock_guard lock{ m_mutex }; + ASSERT(m_inFlightDispatch != 0); + if (--m_inFlightDispatch == 0) + { + m_quiesced.notify_all(); + } + } + + void WaitForQuiesce() noexcept + { + std::unique_lock lock{ m_mutex }; + m_quiesced.wait(lock, [this]() noexcept { return m_inFlightDispatch == 0; }); + } + + void InvokeCallback() noexcept + { + m_callback(m_context); + } + + private: + void* m_context; + WaitTimerCallback* m_callback; + std::atomic m_terminating{ false }; + DefaultUnnamedMutex m_mutex; + DefaultUnnamedConditionVariable m_quiesced; + uint32_t m_inFlightDispatch = 0; + }; + class WaitTimerImpl { public: @@ -37,12 +96,10 @@ namespace OS HRESULT Initialize(_In_opt_ void* context, _In_ WaitTimerCallback* callback); void Start(_In_ uint64_t dueTime); void Cancel(); - void InvokeCallback(); + void Terminate() noexcept; private: - - void* m_context; - WaitTimerCallback* m_callback; + std::shared_ptr m_state; std::shared_ptr m_timerQueue; }; @@ -50,7 +107,9 @@ namespace OS { Deadline When; WaitTimerImpl* Timer; - TimerEntry(Deadline d, WaitTimerImpl* t) : When{ d }, Timer{ t } {} + std::shared_ptr State; + TimerEntry(Deadline d, WaitTimerImpl* timer, std::shared_ptr state) + : When{ d }, Timer{ timer }, State{ std::move(state) } {} }; struct TimerEntryComparator @@ -61,14 +120,23 @@ namespace OS } }; - class TimerQueue + // A single process-wide TimerQueue owns the worker thread that fires every + // WaitTimer callback. Ownership is shared: each live WaitTimerImpl holds a + // shared_ptr, and the running worker holds one too (captured in Init), so + // the queue is never destroyed while a callback is in flight. m_timerCount + // tracks live timers under g_timerQueueMutex so the last owner can retire + // the queue and stop the worker. + class TimerQueue : public std::enable_shared_from_this { public: bool Init() noexcept; ~TimerQueue(); - void Set(WaitTimerImpl* timer, Deadline deadline) noexcept; + void AddTimer() noexcept; + void RemoveTimer() noexcept; + void Set(WaitTimerImpl* timer, std::shared_ptr const& state, Deadline deadline) noexcept; void Remove(WaitTimerImpl const* timer) noexcept; + std::thread::id WorkerThreadId() const noexcept; private: void Worker() noexcept; @@ -80,6 +148,7 @@ namespace OS DefaultUnnamedConditionVariable m_cv; std::vector m_queue; // used as a heap std::thread m_t; + uint32_t m_timerCount = 0; // live timers; guarded by g_timerQueueMutex bool m_exitThread = false; bool m_initialized = false; }; @@ -88,19 +157,90 @@ namespace OS { std::shared_ptr g_timerQueue; DefaultUnnamedMutex g_timerQueueMutex; + DefaultUnnamedMutex g_testHooksMutex; + DefaultUnnamedConditionVariable g_testHooksChanged; + WaitTimerTestHooks* g_testHooks = nullptr; + std::atomic g_testHooksInstalled{ false }; + uint32_t g_testHooksActive = 0; + bool g_testHooksReplacing = false; + } + + class WaitTimerTestHookLease + { + public: + WaitTimerTestHookLease() noexcept + { + // Production fast path: when no hooks are installed, avoid the + // process-global hook mutex entirely on Start/Cancel/dispatch. + if (!g_testHooksInstalled.load(std::memory_order_acquire)) + { + return; + } + + std::unique_lock lock{ g_testHooksMutex }; + g_testHooksChanged.wait(lock, []() noexcept { return !g_testHooksReplacing; }); + m_hooks = g_testHooks; + if (m_hooks != nullptr) + { + ++g_testHooksActive; + } + } + + ~WaitTimerTestHookLease() + { + if (m_hooks != nullptr) + { + std::lock_guard lock{ g_testHooksMutex }; + ASSERT(g_testHooksActive != 0); + if (--g_testHooksActive == 0) + { + g_testHooksChanged.notify_all(); + } + } + } + + WaitTimerTestHooks* Get() const noexcept + { + return m_hooks; + } + + private: + WaitTimerTestHooks* m_hooks = nullptr; + }; + + void WaitTimerSetTestHooks(_In_opt_ WaitTimerTestHooks* hooks) noexcept + { + std::unique_lock lock{ g_testHooksMutex }; + g_testHooksReplacing = true; + g_testHooksChanged.wait(lock, []() noexcept { return g_testHooksActive == 0; }); + g_testHooks = hooks; + g_testHooksInstalled.store(hooks != nullptr, std::memory_order_release); + g_testHooksReplacing = false; + lock.unlock(); + g_testHooksChanged.notify_all(); } TimerQueue::~TimerQueue() { { - std::lock_guard lock{ m_mutex }; + std::lock_guard lock{ m_mutex }; m_exitThread = true; } m_cv.notify_all(); if (m_t.joinable()) { - m_t.join(); + // A timer callback can drop the last queue reference, so the queue + // may be destroyed on its own worker thread. Detach in that case; + // joining our own thread would deadlock. + if (m_t.get_id() == std::this_thread::get_id()) + { + m_t.detach(); + } + else + { + m_t.join(); + } } } @@ -110,9 +250,12 @@ namespace OS try { - m_t = std::thread([this]() + // Capture a shared_ptr so the worker keeps the queue alive for its + // whole lifetime. This lets a callback tear down its own timer (and + // the last queue reference) without the worker touching freed state. + m_t = std::thread([keepAlive = shared_from_this()]() { - Worker(); + keepAlive->Worker(); }); m_initialized = true; } @@ -124,20 +267,65 @@ namespace OS return m_initialized; } - void TimerQueue::Set(WaitTimerImpl* timer, Deadline deadline) noexcept + void TimerQueue::AddTimer() noexcept + { + // Caller holds g_timerQueueMutex, which serializes timer adoption in + // Initialize with last-owner retirement in RemoveTimer. + ++m_timerCount; + } + + void TimerQueue::RemoveTimer() noexcept + { + { + // The decrement and the last-owner decision run under the same lock + // Initialize takes to adopt a timer, so a queue that is retiring can + // never be resurrected by a concurrent Initialize. + std::lock_guard globalLock{ g_timerQueueMutex }; + ASSERT(m_timerCount != 0); + if (--m_timerCount != 0) + { + return; + } + + WaitTimerTestHookLease testHooks; + if (auto hooks = testHooks.Get()) + { + hooks->BeforeTimerQueueRetirement(); + } + + if (g_timerQueue.get() == this) + { + g_timerQueue.reset(); + } + } + + { + std::lock_guard lock{ m_mutex }; + m_exitThread = true; + } + m_cv.notify_all(); + } + + std::thread::id TimerQueue::WorkerThreadId() const noexcept + { + return m_t.get_id(); + } + + void TimerQueue::Set(WaitTimerImpl* timer, std::shared_ptr const& state, Deadline deadline) noexcept { { - std::lock_guard lock{ m_mutex }; + std::lock_guard lock{ m_mutex }; for (auto& entry : m_queue) { if (entry.Timer == timer) { entry.Timer = nullptr; + entry.State.reset(); } } - m_queue.emplace_back(deadline, timer); + m_queue.emplace_back(deadline, timer, state); std::push_heap(m_queue.begin(), m_queue.end(), TimerEntryComparator{}); } m_cv.notify_all(); @@ -145,7 +333,7 @@ namespace OS void TimerQueue::Remove(WaitTimerImpl const* timer) noexcept { - std::lock_guard lock{ m_mutex }; + std::lock_guard lock{ m_mutex }; // since m_queue is a heap, removing elements is non trivial, instead we // just clean the timer pointer and the entry will be popped eventually @@ -155,13 +343,14 @@ namespace OS if (entry.Timer == timer) { entry.Timer = nullptr; + entry.State.reset(); } } } void TimerQueue::Worker() noexcept { - std::unique_lock lock{ m_mutex }; + std::unique_lock lock{ m_mutex }; while (!m_exitThread) { while (!m_queue.empty()) @@ -174,12 +363,17 @@ namespace OS TimerEntry entry = Pop(); - // release the lock while invoking the callback, just in case timer - // gets destroyed on this thread or re-adds itself in the callback + // The raw timer pointer is only a cancellation key. Dispatch owns + // the shared state so teardown cannot invalidate the callback. lock.unlock(); - if (entry.Timer) // Timer is set to nullptr if the entry is removed + if (entry.State && entry.State->TryBeginDispatch()) { - entry.Timer->InvokeCallback(); + WaitTimerTestHookLease testHooks; + if (auto hooks = testHooks.Get(); hooks == nullptr || hooks->BeforeTimerInvoke()) + { + entry.State->InvokeCallback(); + } + entry.State->EndDispatch(); } lock.lock(); } @@ -213,27 +407,58 @@ namespace OS WaitTimerImpl::~WaitTimerImpl() { - std::lock_guard lock{ g_timerQueueMutex }; + Terminate(); + } + + void WaitTimerImpl::Terminate() noexcept + { + // Take ownership of the shared state and queue. Terminate is idempotent: + // the destructor always runs it, and a second call is a no-op once the + // members have been moved out. + std::shared_ptr state = std::move(m_state); + std::shared_ptr timerQueue = std::move(m_timerQueue); + + // Initialize may have failed before wiring up state/queue; there is + // nothing to tear down in that case. + if (state == nullptr || timerQueue == nullptr) + { + return; + } - // If we are the last one referencing the global timer the - // shared use count will be two (us + the global). If it is, - // clear out the global. We let our own reference reset - // as the class destructs. This puts it outside the mutex - // lock, which we want since there is some shutdown cost - // associated with shutting the timer down. + // Stop new dispatches, drop any queued entry, then wait for an in-flight + // dispatch to finish so the callback can never run against freed state. + // Skip the wait on the worker thread (a callback is tearing down its own + // timer): blocking on our own dispatch would deadlock, and the entry's + // shared-state lease keeps the state alive until the callback returns. + state->BeginTerminate(); + timerQueue->Remove(this); + if (std::this_thread::get_id() != timerQueue->WorkerThreadId()) + { + state->WaitForQuiesce(); + } - if (g_timerQueue.use_count() == 2) + WaitTimerTestHookLease testHooks; + if (auto hooks = testHooks.Get()) { - g_timerQueue.reset(); + hooks->WaitTimerImplDestructionStarted(); } + + // Release this timer's queue ownership; the last owner retires the queue. + timerQueue->RemoveTimer(); } HRESULT WaitTimerImpl::Initialize(_In_opt_ void* context, _In_ WaitTimerCallback* callback) { - m_context = context; - m_callback = callback; + try + { + m_state = http_allocate_shared(context, callback); + } + catch (const std::bad_alloc&) + { + return E_OUTOFMEMORY; + } - std::lock_guard lock{ g_timerQueueMutex }; + std::lock_guard lock{ g_timerQueueMutex }; if (g_timerQueue == nullptr) { @@ -254,13 +479,14 @@ namespace OS } m_timerQueue = g_timerQueue; + m_timerQueue->AddTimer(); return S_OK; } void WaitTimerImpl::Start(_In_ uint64_t dueTime) { - m_timerQueue->Set(this, Deadline(Deadline::duration(dueTime))); + m_timerQueue->Set(this, m_state, Deadline(Deadline::duration(dueTime))); } void WaitTimerImpl::Cancel() @@ -268,11 +494,6 @@ namespace OS m_timerQueue->Remove(this); } - void WaitTimerImpl::InvokeCallback() - { - m_callback(m_context); - } - WaitTimer::WaitTimer() noexcept : m_impl(nullptr) {} @@ -284,6 +505,7 @@ namespace OS HRESULT WaitTimer::Initialize(_In_opt_ void* context, _In_ WaitTimerCallback* callback) noexcept { + std::lock_guard lock{ m_mutex }; if (m_impl.load() != nullptr || callback == nullptr) { ASSERT(false); @@ -301,7 +523,11 @@ namespace OS void WaitTimer::Terminate() noexcept { - std::unique_ptr timer(m_impl.exchange(nullptr)); + std::unique_ptr timer; + { + std::lock_guard lock{ m_mutex }; + timer.reset(m_impl.exchange(nullptr)); + } if (timer != nullptr) { timer->Cancel(); @@ -310,12 +536,32 @@ namespace OS void WaitTimer::Start(_In_ uint64_t dueTime) noexcept { - m_impl.load()->Start(dueTime); + std::lock_guard lock{ m_mutex }; + WaitTimerImpl* timer = m_impl.load(); + WaitTimerTestHookLease testHooks; + if (auto hooks = testHooks.Get(); hooks != nullptr && !hooks->WaitTimerOperationLoaded(true)) + { + return; + } + if (timer != nullptr) + { + timer->Start(dueTime); + } } void WaitTimer::Cancel() noexcept { - m_impl.load()->Cancel(); + std::lock_guard lock{ m_mutex }; + WaitTimerImpl* timer = m_impl.load(); + WaitTimerTestHookLease testHooks; + if (auto hooks = testHooks.Get(); hooks != nullptr && !hooks->WaitTimerOperationLoaded(false)) + { + return; + } + if (timer != nullptr) + { + timer->Cancel(); + } } uint64_t WaitTimer::GetCurrentTime() noexcept diff --git a/Source/Task/WaitTimer_win32.cpp b/Source/Task/WaitTimer_win32.cpp index 716dcf16..8f97a6fa 100644 --- a/Source/Task/WaitTimer_win32.cpp +++ b/Source/Task/WaitTimer_win32.cpp @@ -126,6 +126,7 @@ namespace OS HRESULT WaitTimer::Initialize(_In_opt_ void* context, _In_ WaitTimerCallback* callback) noexcept { + std::lock_guard lock{ m_mutex }; if (m_impl.load() != nullptr || callback == nullptr) { ASSERT(FALSE); @@ -143,7 +144,11 @@ namespace OS void WaitTimer::Terminate() noexcept { - std::unique_ptr timer(m_impl.exchange(nullptr)); + std::unique_ptr timer; + { + std::lock_guard lock{ m_mutex }; + timer.reset(m_impl.exchange(nullptr)); + } if (timer != nullptr) { timer->Terminate(); @@ -152,12 +157,20 @@ namespace OS void WaitTimer::Start(_In_ uint64_t dueTime) noexcept { - m_impl.load()->Start(dueTime); + std::lock_guard lock{ m_mutex }; + if (WaitTimerImpl* timer = m_impl.load()) + { + timer->Start(dueTime); + } } void WaitTimer::Cancel() noexcept { - m_impl.load()->Cancel(); + std::lock_guard lock{ m_mutex }; + if (WaitTimerImpl* timer = m_impl.load()) + { + timer->Cancel(); + } } uint64_t WaitTimer::GetCurrentTime() noexcept diff --git a/Source/Task/iOS/ios_WaitTimer.mm b/Source/Task/iOS/ios_WaitTimer.mm index 53257ebe..c7d0409e 100644 --- a/Source/Task/iOS/ios_WaitTimer.mm +++ b/Source/Task/iOS/ios_WaitTimer.mm @@ -100,6 +100,7 @@ uint64_t DueTimeFromDeadline(Deadline deadline) noexcept HRESULT WaitTimer::Initialize(_In_opt_ void* context, _In_ WaitTimerCallback* callback) noexcept { + std::lock_guard lock{ m_mutex }; if (m_impl.load() != nullptr || callback == nullptr) { ASSERT(false); @@ -117,7 +118,11 @@ uint64_t DueTimeFromDeadline(Deadline deadline) noexcept void WaitTimer::Terminate() noexcept { - WaitTimerImpl* timer = m_impl.exchange(nullptr); + WaitTimerImpl* timer; + { + std::lock_guard lock{ m_mutex }; + timer = m_impl.exchange(nullptr); + } if (timer != nullptr) { timer->Cancel(); @@ -127,12 +132,20 @@ uint64_t DueTimeFromDeadline(Deadline deadline) noexcept void WaitTimer::Start(_In_ uint64_t dueTime) noexcept { - m_impl.load()->Start(dueTime); + std::lock_guard lock{ m_mutex }; + if (WaitTimerImpl* timer = m_impl.load()) + { + timer->Start(dueTime); + } } void WaitTimer::Cancel() noexcept { - m_impl.load()->Cancel(); + std::lock_guard lock{ m_mutex }; + if (WaitTimerImpl* timer = m_impl.load()) + { + timer->Cancel(); + } } uint64_t WaitTimer::GetCurrentTime() noexcept diff --git a/Tests/TaskQueueStarvation/SingleQueuePollStrandRepro.cpp b/Tests/StandaloneTests/SingleQueuePollStrandRepro.cpp similarity index 100% rename from Tests/TaskQueueStarvation/SingleQueuePollStrandRepro.cpp rename to Tests/StandaloneTests/SingleQueuePollStrandRepro.cpp diff --git a/Tests/TaskQueueStarvation/TaskQueueStarvationRepro.cpp b/Tests/StandaloneTests/TaskQueueStarvationRepro.cpp similarity index 100% rename from Tests/TaskQueueStarvation/TaskQueueStarvationRepro.cpp rename to Tests/StandaloneTests/TaskQueueStarvationRepro.cpp diff --git a/Tests/StandaloneTests/WaitTimerQueueRetirementRepro.cpp b/Tests/StandaloneTests/WaitTimerQueueRetirementRepro.cpp new file mode 100644 index 00000000..2bb682c1 --- /dev/null +++ b/Tests/StandaloneTests/WaitTimerQueueRetirementRepro.cpp @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// Deterministic regression for last-owner retirement racing a new timer. This +// target compiles Source/Task/WaitTimer_stl.cpp directly so it can pause inside +// TimerQueue::RemoveTimer, exactly while the retiring owner holds +// g_timerQueueMutex, and prove a concurrent Initialize cannot resurrect a queue +// that is being retired. + +#include +#include "../../Source/Task/WaitTimer.h" + +#include +#include +#include +#include +#include +#include + +namespace +{ + using namespace std::chrono_literals; + + class RetirementHooks final : public OS::WaitTimerTestHooks + { + public: + void BeforeTimerQueueRetirement() noexcept override + { + std::unique_lock lock(m_mutex); + m_retirementStarted = true; + m_changed.notify_all(); + m_changed.wait(lock, [this]() noexcept { return m_releaseRetirement; }); + } + + bool WaitForRetirement() + { + std::unique_lock lock(m_mutex); + return m_changed.wait_for(lock, 5s, [this]() noexcept { return m_retirementStarted; }); + } + + void ReleaseRetirement() + { + std::lock_guard lock(m_mutex); + m_releaseRetirement = true; + m_changed.notify_all(); + } + + private: + std::mutex m_mutex; + std::condition_variable m_changed; + bool m_retirementStarted = false; + bool m_releaseRetirement = false; + }; + + struct CallbackState + { + std::mutex Mutex; + std::condition_variable Changed; + bool Ran = false; + }; + + void CALLBACK TimerCallback(void* context) + { + auto callbackState = static_cast(context); + { + std::lock_guard lock(callbackState->Mutex); + callbackState->Ran = true; + } + callbackState->Changed.notify_all(); + } +} + +int main() +{ + RetirementHooks hooks; + OS::WaitTimerSetTestHooks(&hooks); + + OS::WaitTimer retiringTimer; + HRESULT hr = retiringTimer.Initialize(nullptr, [](void*) {}); + if (FAILED(hr)) + { + std::printf("[waittimer-queue-retirement] FAILED: first Initialize returned 0x%08x\n", static_cast(hr)); + OS::WaitTimerSetTestHooks(nullptr); + return 2; + } + + std::thread retire([&retiringTimer]() { retiringTimer.Terminate(); }); + if (!hooks.WaitForRetirement()) + { + std::printf("[waittimer-queue-retirement] FAILED: timer queue did not begin last-owner retirement\n"); + hooks.ReleaseRetirement(); + retire.join(); + OS::WaitTimerSetTestHooks(nullptr); + return 2; + } + + CallbackState callbackState; + OS::WaitTimer newTimer; + std::atomic initialized{ false }; + std::thread initialize([&newTimer, &callbackState, &initialized]() + { + initialized.store(SUCCEEDED(newTimer.Initialize(&callbackState, TimerCallback)), std::memory_order_release); + }); + + hooks.ReleaseRetirement(); + retire.join(); + initialize.join(); + OS::WaitTimerSetTestHooks(nullptr); + + if (!initialized.load(std::memory_order_acquire)) + { + std::printf("[waittimer-queue-retirement] FAILED: concurrent Initialize did not succeed\n"); + return 2; + } + + newTimer.Start(newTimer.GetDueTime(1)); + std::unique_lock lock(callbackState.Mutex); + if (!callbackState.Changed.wait_for(lock, 5s, [&callbackState]() noexcept { return callbackState.Ran; })) + { + std::printf("[waittimer-queue-retirement] FAILED: newly initialized timer did not dispatch\n"); + return 1; + } + + newTimer.Terminate(); + std::printf("[waittimer-queue-retirement] PASSED: concurrent initialization received a live timer queue\n"); + return 0; +} diff --git a/Tests/StandaloneTests/WaitTimerQueueTeardownRepro.cpp b/Tests/StandaloneTests/WaitTimerQueueTeardownRepro.cpp new file mode 100644 index 00000000..1c1563aa --- /dev/null +++ b/Tests/StandaloneTests/WaitTimerQueueTeardownRepro.cpp @@ -0,0 +1,147 @@ +// Copyright (c) Microsoft Corporation +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// Deterministic regression for the STL WaitTimer worker-pop teardown race. +// This target compiles Source/Task/WaitTimer_stl.cpp directly, rather than +// attempting to drive the race through public XTaskQueue APIs. The public API +// cannot reliably stop the worker after it has copied its internal timer +// pointer and released TimerQueue's lock. + +#include +#include "../../Source/Task/WaitTimer.h" + +#include +#include +#include +#include +#include +#include + +namespace +{ + using namespace std::chrono_literals; + + class QueueTeardownHooks final : public OS::WaitTimerTestHooks + { + public: + bool BeforeTimerInvoke() noexcept override + { + std::unique_lock lock(m_mutex); + m_timerPopped = true; + m_changed.notify_all(); + m_changed.wait(lock, [this]() noexcept { return m_releaseWorker; }); + return false; + } + + void WaitTimerImplDestructionStarted() noexcept override + { + std::lock_guard lock(m_mutex); + m_destructionStarted = true; + m_changed.notify_all(); + } + + bool WaitForTimerPop() + { + std::unique_lock lock(m_mutex); + return m_changed.wait_for(lock, 5s, [this]() noexcept { return m_timerPopped; }); + } + + bool DestructionStartedWithin(std::chrono::milliseconds timeout) + { + std::unique_lock lock(m_mutex); + return m_changed.wait_for(lock, timeout, [this]() noexcept { return m_destructionStarted; }); + } + + bool WaitForDestruction() + { + return DestructionStartedWithin(5s); + } + + void ReleaseWorker() + { + std::lock_guard lock(m_mutex); + m_releaseWorker = true; + m_changed.notify_all(); + } + + private: + std::mutex m_mutex; + std::condition_variable m_changed; + bool m_timerPopped = false; + bool m_destructionStarted = false; + bool m_releaseWorker = false; + }; +} + +int main() +{ + QueueTeardownHooks hooks; + OS::WaitTimerSetTestHooks(&hooks); + + OS::WaitTimer timer; + HRESULT hr = timer.Initialize(nullptr, [](void*) {}); + if (FAILED(hr)) + { + std::printf("[waittimer-queue-teardown] FAILED: Initialize returned 0x%08x\n", static_cast(hr)); + OS::WaitTimerSetTestHooks(nullptr); + return 2; + } + + // Keep the shared TimerQueue alive while the target timer is destroyed. + // Otherwise the old implementation synchronously joins the paused worker + // when retiring the final queue owner, obscuring the dispatch race. + OS::WaitTimer queueOwner; + hr = queueOwner.Initialize(nullptr, [](void*) {}); + if (FAILED(hr)) + { + std::printf("[waittimer-queue-teardown] FAILED: keepalive Initialize returned 0x%08x\n", static_cast(hr)); + timer.Terminate(); + OS::WaitTimerSetTestHooks(nullptr); + return 2; + } + + timer.Start(timer.GetDueTime(1)); + if (!hooks.WaitForTimerPop()) + { + std::printf("[waittimer-queue-teardown] FAILED: timer worker did not pop the due entry\n"); + timer.Terminate(); + queueOwner.Terminate(); + OS::WaitTimerSetTestHooks(nullptr); + return 2; + } + + std::atomic terminateComplete{ false }; + std::thread terminate([&]() + { + timer.Terminate(); + terminateComplete.store(true, std::memory_order_release); + }); + + const bool destructionStartedWhileWorkerBlocked = hooks.DestructionStartedWithin(100ms); + hooks.ReleaseWorker(); + terminate.join(); + const bool destructionStarted = hooks.WaitForDestruction(); + queueOwner.Terminate(); + OS::WaitTimerSetTestHooks(nullptr); + + if (!destructionStarted) + { + std::printf("[waittimer-queue-teardown] FAILED: WaitTimer implementation was not destroyed\n"); + return 2; + } + + if (destructionStartedWhileWorkerBlocked) + { + std::printf("[waittimer-queue-teardown] UNSAFE: implementation destruction began after worker copied its raw timer pointer\n"); + return 1; + } + + if (!terminateComplete.load(std::memory_order_acquire)) + { + std::printf("[waittimer-queue-teardown] FAILED: Terminate did not complete\n"); + return 2; + } + + std::printf("[waittimer-queue-teardown] PASSED: dispatch lifetime outlived worker teardown\n"); + return 0; +} \ No newline at end of file diff --git a/Tests/StandaloneTests/WaitTimerTestMemory.cpp b/Tests/StandaloneTests/WaitTimerTestMemory.cpp new file mode 100644 index 00000000..a69bb6c6 --- /dev/null +++ b/Tests/StandaloneTests/WaitTimerTestMemory.cpp @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft Corporation +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// Standalone white-box WaitTimer tests compile the private implementation into +// their executable. Its allocator helpers are deliberately hidden from the +// shared-library ABI, so these tests provide the default allocator locally. + +#include "../../Source/Common/pch.h" +#include "../../Source/Global/mem.h" + +#include + +NAMESPACE_XBOX_HTTP_CLIENT_BEGIN + +void* http_memory::mem_alloc(size_t size) +{ + return std::malloc(size); +} + +void http_memory::mem_free(void* address) +{ + std::free(address); +} + +NAMESPACE_XBOX_HTTP_CLIENT_END \ No newline at end of file diff --git a/Tests/StandaloneTests/WaitTimerWrapperTerminateRepro.cpp b/Tests/StandaloneTests/WaitTimerWrapperTerminateRepro.cpp new file mode 100644 index 00000000..c7b6f0d5 --- /dev/null +++ b/Tests/StandaloneTests/WaitTimerWrapperTerminateRepro.cpp @@ -0,0 +1,161 @@ +// Copyright (c) Microsoft Corporation +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// Deterministic WaitTimer wrapper lifetime regression. This target compiles +// Source/Task/WaitTimer_stl.cpp directly so it can pause exactly after Start +// or Cancel reads the private m_impl pointer. Public XTaskQueue APIs and their +// hooks cannot expose or reliably schedule this wrapper-only interleaving. + +#include +#include "../../Source/Task/WaitTimer.h" + +#include +#include +#include +#include +#include + +namespace +{ + using namespace std::chrono_literals; + + class WrapperHooks final : public OS::WaitTimerTestHooks + { + public: + bool WaitTimerOperationLoaded(bool isStart) noexcept override + { + std::unique_lock lock(m_mutex); + m_operationLoaded = true; + m_isStart = isStart; + m_changed.notify_all(); + m_changed.wait(lock, [this]() noexcept { return m_releaseOperation; }); + m_operationResumedAfterDestruction = m_destructionStarted; + m_changed.notify_all(); + + // Avoid dereferencing the stale pointer after proving the race. + return !m_operationResumedAfterDestruction; + } + + void WaitTimerImplDestructionStarted() noexcept override + { + std::lock_guard lock(m_mutex); + m_destructionStarted = true; + m_changed.notify_all(); + } + + bool WaitForOperation(bool isStart) + { + std::unique_lock lock(m_mutex); + return m_changed.wait_for(lock, 5s, [this, isStart]() noexcept + { + return m_operationLoaded && m_isStart == isStart; + }); + } + + bool DestructionStartedWithin(std::chrono::milliseconds timeout) + { + std::unique_lock lock(m_mutex); + return m_changed.wait_for(lock, timeout, [this]() noexcept { return m_destructionStarted; }); + } + + bool WaitForDestruction() + { + return DestructionStartedWithin(5s); + } + + void ReleaseOperation() + { + std::lock_guard lock(m_mutex); + m_releaseOperation = true; + m_changed.notify_all(); + } + + bool OperationResumedAfterDestruction() + { + std::lock_guard lock(m_mutex); + return m_operationResumedAfterDestruction; + } + + private: + std::mutex m_mutex; + std::condition_variable m_changed; + bool m_operationLoaded = false; + bool m_isStart = false; + bool m_releaseOperation = false; + bool m_destructionStarted = false; + bool m_operationResumedAfterDestruction = false; + }; + + bool RunCase(bool isStart) + { + WrapperHooks hooks; + OS::WaitTimerSetTestHooks(&hooks); + + OS::WaitTimer timer; + HRESULT hr = timer.Initialize(nullptr, [](void*) {}); + if (FAILED(hr)) + { + std::printf("[waittimer-wrapper-terminate] FAILED: Initialize returned 0x%08x\n", static_cast(hr)); + OS::WaitTimerSetTestHooks(nullptr); + return false; + } + + std::thread operation([&timer, isStart]() + { + if (isStart) + { + timer.Start(timer.GetDueTime(60000)); + } + else + { + timer.Cancel(); + } + }); + + if (!hooks.WaitForOperation(isStart)) + { + std::printf("[waittimer-wrapper-terminate] FAILED: %s did not observe m_impl\n", isStart ? "Start" : "Cancel"); + hooks.ReleaseOperation(); + operation.join(); + timer.Terminate(); + OS::WaitTimerSetTestHooks(nullptr); + return false; + } + + std::thread terminate([&timer]() { timer.Terminate(); }); + const bool destructionStartedWhileOperationBlocked = hooks.DestructionStartedWithin(100ms); + hooks.ReleaseOperation(); + operation.join(); + terminate.join(); + const bool unsafe = hooks.OperationResumedAfterDestruction(); + const bool destructionStarted = hooks.WaitForDestruction(); + OS::WaitTimerSetTestHooks(nullptr); + + if (destructionStartedWhileOperationBlocked || unsafe) + { + std::printf("[waittimer-wrapper-terminate] UNSAFE: implementation destruction began while %s held its raw implementation reference\n", isStart ? "Start" : "Cancel"); + return false; + } + + if (!destructionStarted) + { + std::printf("[waittimer-wrapper-terminate] FAILED: implementation destruction did not run after %s completed\n", isStart ? "Start" : "Cancel"); + return false; + } + + return true; + } +} + +int main() +{ + const bool startPassed = RunCase(true); + const bool cancelPassed = RunCase(false); + if (!startPassed || !cancelPassed) + { + return 1; + } + + std::printf("[waittimer-wrapper-terminate] PASSED: Start and Cancel cannot race implementation teardown\n"); + return 0; +} \ No newline at end of file From a94b99e9c0c6db44cbce9163ad5ec2d639b6747c Mon Sep 17 00:00:00 2001 From: Pushpadant Kacha Date: Mon, 10 Aug 2026 14:25:29 -0700 Subject: [PATCH 2/3] Integrate WaitTimer lifetime fix with PS5 unnamed mutexes WaitTimer #1005 protects its state with DefaultUnnamedMutex. On stable, #1011 makes that a distinct std::mutex-derived type on PS5, so std::condition_variable cannot accept unique_lock. Use DefaultUnnamedConditionVariableAny for the three WaitTimer condition variables paired with DefaultUnnamedMutex. This preserves unnamed PS5 synchronization objects and compiles on both PS5 and STL platforms. --- Source/Task/WaitTimer_stl.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Task/WaitTimer_stl.cpp b/Source/Task/WaitTimer_stl.cpp index be4f8940..c3fdc98d 100644 --- a/Source/Task/WaitTimer_stl.cpp +++ b/Source/Task/WaitTimer_stl.cpp @@ -85,7 +85,7 @@ namespace OS WaitTimerCallback* m_callback; std::atomic m_terminating{ false }; DefaultUnnamedMutex m_mutex; - DefaultUnnamedConditionVariable m_quiesced; + DefaultUnnamedConditionVariableAny m_quiesced; uint32_t m_inFlightDispatch = 0; }; @@ -145,7 +145,7 @@ namespace OS TimerEntry Pop() noexcept; DefaultUnnamedMutex m_mutex; - DefaultUnnamedConditionVariable m_cv; + DefaultUnnamedConditionVariableAny m_cv; std::vector m_queue; // used as a heap std::thread m_t; uint32_t m_timerCount = 0; // live timers; guarded by g_timerQueueMutex @@ -158,7 +158,7 @@ namespace OS std::shared_ptr g_timerQueue; DefaultUnnamedMutex g_timerQueueMutex; DefaultUnnamedMutex g_testHooksMutex; - DefaultUnnamedConditionVariable g_testHooksChanged; + DefaultUnnamedConditionVariableAny g_testHooksChanged; WaitTimerTestHooks* g_testHooks = nullptr; std::atomic g_testHooksInstalled{ false }; uint32_t g_testHooksActive = 0; From b3a848e893c9931badb0b24a2e0ea2336294cc5b Mon Sep 17 00:00:00 2001 From: Pushpadant Kacha Date: Mon, 10 Aug 2026 14:41:08 -0700 Subject: [PATCH 3/3] Add public manual-queue timer retirement regression Combine the deterministic WaitTimer retirement hook with public XTaskQueueCreate(Manual, Manual), queue destruction, replacement creation, and delayed dispatch. This covers components that recreate private task queues during lifecycle resets and ensures the replacement queue cannot adopt a retiring process-wide timer worker. --- Build/libHttpClient.Linux/CMakeLists.txt | 58 +++++++ .../ManualQueueTimerRetirementRepro.cpp | 162 ++++++++++++++++++ 2 files changed, 220 insertions(+) create mode 100644 Tests/StandaloneTests/ManualQueueTimerRetirementRepro.cpp diff --git a/Build/libHttpClient.Linux/CMakeLists.txt b/Build/libHttpClient.Linux/CMakeLists.txt index 93e4b797..d6b82854 100644 --- a/Build/libHttpClient.Linux/CMakeLists.txt +++ b/Build/libHttpClient.Linux/CMakeLists.txt @@ -489,6 +489,64 @@ set_tests_properties( LABELS "taskqueue" ) +################################################# +### Manual queue timer-retirement integration ### +################################################# +# Public-XTaskQueue regression for a component destroying its private +# Manual/Manual queue while creating the replacement. The internal retirement +# hook makes the lifetime race deterministic; the delayed callback proves the +# replacement queue received a live STL timer worker. The hook is intentionally +# not exported from the shared library, so this test belongs to the static-build +# test pass (the Linux pipeline runs ctest immediately after that build). +if (NOT BUILD_SHARED_LIBS) +add_executable( + "ManualQueueTimerRetirementTests.Linux" + "${PATH_TO_ROOT}/Tests/StandaloneTests/ManualQueueTimerRetirementRepro.cpp" +) + +target_include_directories( + "ManualQueueTimerRetirementTests.Linux" + PRIVATE + "${COMMON_INCLUDE_DIRS}" + "${LINUX_INCLUDE_DIRS}" +) + +target_link_libraries( + "ManualQueueTimerRetirementTests.Linux" + PRIVATE + "${PROJECT_NAME}" + Threads::Threads + ${CMAKE_DL_LIBS} +) + +if (NOT BUILD_SHARED_LIBS) + target_link_libraries( + "ManualQueueTimerRetirementTests.Linux" + PRIVATE + "${LIBCURL_BINARY_PATH}" + "${LIBSSL_BINARY_PATH}" + "${LIBCRYPTO_BINARY_PATH}" + ) +endif() + +target_set_flags( + "ManualQueueTimerRetirementTests.Linux" + "${FLAGS}" + "${FLAGS_DEBUG}" + "${FLAGS_RELEASE}" +) + +add_test( + NAME manual-queue-timer-retirement-linux + COMMAND "ManualQueueTimerRetirementTests.Linux" +) +set_tests_properties( + manual-queue-timer-retirement-linux + PROPERTIES + LABELS "taskqueue" +) +endif() + ################################################# ### WaitTimer wrapper lifetime race repro ### ################################################# diff --git a/Tests/StandaloneTests/ManualQueueTimerRetirementRepro.cpp b/Tests/StandaloneTests/ManualQueueTimerRetirementRepro.cpp new file mode 100644 index 00000000..07bc311f --- /dev/null +++ b/Tests/StandaloneTests/ManualQueueTimerRetirementRepro.cpp @@ -0,0 +1,162 @@ +// Copyright (c) Microsoft Corporation +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// Public-XTaskQueue integration regression for last-owner WaitTimer retirement. +// A private Manual/Manual queue is destroyed while its replacement is created, +// matching components that recreate a dedicated queue during lifecycle resets. +// The replacement queue must receive a live timer worker and dispatch delayed work. + +#include +#include "../../Source/Task/WaitTimer.h" + +#include +#include +#include +#include +#include +#include + +namespace +{ + using namespace std::chrono_literals; + + class RetirementHooks final : public OS::WaitTimerTestHooks + { + public: + void BeforeTimerQueueRetirement() noexcept override + { + std::unique_lock lock{ m_mutex }; + m_retirementStarted = true; + m_changed.notify_all(); + m_changed.wait(lock, [this]() noexcept { return m_releaseRetirement; }); + } + + bool WaitForRetirement() + { + std::unique_lock lock{ m_mutex }; + return m_changed.wait_for(lock, 5s, [this]() noexcept { return m_retirementStarted; }); + } + + void ReleaseRetirement() + { + std::lock_guard lock{ m_mutex }; + m_releaseRetirement = true; + m_changed.notify_all(); + } + + private: + std::mutex m_mutex; + std::condition_variable m_changed; + bool m_retirementStarted = false; + bool m_releaseRetirement = false; + }; + + void CALLBACK DelayedCallback(void* context, bool canceled) + { + if (!canceled) + { + static_cast*>(context)->store(true, std::memory_order_release); + } + } + + void DrainAndClose(XTaskQueueHandle queue) + { + XTaskQueueTerminate(queue, false, nullptr, nullptr); + while (XTaskQueueDispatch(queue, XTaskQueuePort::Work, 0)) {} + while (XTaskQueueDispatch(queue, XTaskQueuePort::Completion, 0)) {} + XTaskQueueCloseHandle(queue); + } +} + +int main() +{ + RetirementHooks hooks; + OS::WaitTimerSetTestHooks(&hooks); + + XTaskQueueHandle oldQueue{ nullptr }; + HRESULT hr = XTaskQueueCreate( + XTaskQueueDispatchMode::Manual, + XTaskQueueDispatchMode::Manual, + &oldQueue); + if (FAILED(hr)) + { + std::printf("[manual-queue-timer-retirement] FAILED: old queue creation returned 0x%08x\n", static_cast(hr)); + OS::WaitTimerSetTestHooks(nullptr); + return 2; + } + + std::thread closeOldQueue([oldQueue]() { XTaskQueueCloseHandle(oldQueue); }); + if (!hooks.WaitForRetirement()) + { + std::printf("[manual-queue-timer-retirement] FAILED: closing the old queue did not begin timer retirement\n"); + hooks.ReleaseRetirement(); + closeOldQueue.join(); + OS::WaitTimerSetTestHooks(nullptr); + return 2; + } + + XTaskQueueHandle replacementQueue{ nullptr }; + std::atomic creationCompleted{ false }; + HRESULT creationHr = E_FAIL; + std::thread createReplacement([&]() + { + creationHr = XTaskQueueCreate( + XTaskQueueDispatchMode::Manual, + XTaskQueueDispatchMode::Manual, + &replacementQueue); + creationCompleted.store(true, std::memory_order_release); + }); + + // Retirement owns the process-global timer-queue lock. Replacement queue + // initialization must not complete by adopting that retiring queue. + std::this_thread::sleep_for(100ms); + const bool creationEscapedRetirement = creationCompleted.load(std::memory_order_acquire); + + hooks.ReleaseRetirement(); + closeOldQueue.join(); + createReplacement.join(); + OS::WaitTimerSetTestHooks(nullptr); + + if (creationEscapedRetirement) + { + std::printf("[manual-queue-timer-retirement] FAILED: replacement queue adopted a timer queue while it was retiring\n"); + if (replacementQueue != nullptr) + { + DrainAndClose(replacementQueue); + } + return 1; + } + + if (FAILED(creationHr)) + { + std::printf("[manual-queue-timer-retirement] FAILED: replacement queue creation returned 0x%08x\n", static_cast(creationHr)); + return 2; + } + + std::atomic callbackRan{ false }; + hr = XTaskQueueSubmitDelayedCallback( + replacementQueue, + XTaskQueuePort::Work, + 1, + &callbackRan, + DelayedCallback); + if (FAILED(hr)) + { + std::printf("[manual-queue-timer-retirement] FAILED: delayed callback submission returned 0x%08x\n", static_cast(hr)); + DrainAndClose(replacementQueue); + return 2; + } + + const bool dispatched = XTaskQueueDispatch(replacementQueue, XTaskQueuePort::Work, 5000); + const bool callbackObserved = callbackRan.load(std::memory_order_acquire); + DrainAndClose(replacementQueue); + + if (!dispatched || !callbackObserved) + { + std::printf("[manual-queue-timer-retirement] FAILED: replacement queue delayed callback did not dispatch\n"); + return 1; + } + + std::printf("[manual-queue-timer-retirement] PASSED: replacement manual queue received a live timer worker\n"); + return 0; +} \ No newline at end of file