From e1a78b3985cbe1fa214b534b8275e1e763440c67 Mon Sep 17 00:00:00 2001 From: Michael Vandeberg Date: Fri, 23 Oct 2026 12:41:16 -0600 Subject: [PATCH] fix: defer await_ready into await_suspend on affected MSVC builds On MSVC 14.51 ARM64 release, the void-returning symmetric_transfer does not prevent every lost catch. With a call before a try, and an awaiter whose await_ready performs an atomic read-modify-write (as async_waker's does to consume a latched wakeup), a throw inside the try escaped the catch beside it and reached unhandled_exception. A plain or atomic load in await_ready did not trigger it, nor did anything tested on 14.44. Where BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND is on, task's and quitter's transform_awaiter now report not ready, and ask the wrapped await_ready from inside await_suspend through the new detail::await_ready_then_suspend. It calls await_ready exactly once, since it may consume state, and returns false to continue without suspending when the awaitable is ready, so immediate completions do not grow the stack. Builds without the workaround are unchanged. Add regression tests: testCatchAfterRmwAwaitReady and testCatchAroundWakerWait in task.cpp, and testCatchAfterRmwAwaitReady and testCatchStopAfterRmwAwaitReady in quitter.cpp. Without the fix all four fail on MSVC 14.51 ARM64 release and pass on ARM64 14.44, x64 14.44 and x64 14.51. testVoidSuspendAwaitable covers the helper's not-ready, void await_suspend branch. Correct the gate comments: the handle-return defect on ARM64 also reproduces on 19.44, where the 19.34-19.44 clause already covers it. --- .../capy/detail/await_suspend_helper.hpp | 87 +++++++- include/boost/capy/detail/config.hpp | 10 +- include/boost/capy/quitter.hpp | 31 ++- include/boost/capy/task.hpp | 30 ++- test/unit/quitter.cpp | 134 ++++++++++++ test/unit/task.cpp | 198 ++++++++++++++++++ 6 files changed, 470 insertions(+), 20 deletions(-) diff --git a/include/boost/capy/detail/await_suspend_helper.hpp b/include/boost/capy/detail/await_suspend_helper.hpp index eb4672b59..97c7ea159 100644 --- a/include/boost/capy/detail/await_suspend_helper.hpp +++ b/include/boost/capy/detail/await_suspend_helper.hpp @@ -60,15 +60,24 @@ namespace detail { On ARM64 the workaround does not retire at 19.50, because that target has a second, unrelated defect. With a real - symmetric transfer, MSVC 19.51 release loses the handler for - a `try` region that spans the suspend point: after the - coroutine is resumed from another call stack, a `throw` + symmetric transfer, MSVC ARM64 release loses the handler for + a `try` region that spans the suspend point: a `throw` inside that region is not caught by the `catch` beside it and escapes to the promise's `unhandled_exception`. A `catch(...)` misses it too, so the region is not found at all rather than - the handler failing to match. Debug builds are unaffected, as - is x64 at the same toolset. `testCatchAfterDeferredResume` in - test/unit/task.cpp covers this. + the handler failing to match. Any `await_suspend` that returns + a handle triggers it, whether the coroutine resumes at once + or later from another call stack, and whether the throw comes + from `await_resume` or from the body after it. A standalone + program without capy reproduces it on 19.44 and 19.51; the + first clause of the gate already covers 19.44. Debug builds + are unaffected, as is x64 at the same toolset. + `testCatchAfterDeferredResume` in test/unit/task.cpp covers + this. + + The void-returning transfer alone does not cover every case on + 19.51. See `await_ready_then_suspend` below for the second + safeguard that `task` and `quitter` apply where this one is on. `_M_ARM64EC` is included conservatively. It generates ARM64 code and has not been tested here; keeping the workaround on @@ -112,6 +121,72 @@ symmetric_transfer(std::coroutine_handle<> h) noexcept } #endif +#if BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND +/** Ask whether an awaitable is ready, and suspend on it if not. + + Used by the `transform_awaiter` of `task` and `quitter` where the + symmetric-transfer workaround is on. There the awaiter's own + `await_ready` returns `false`, and its `await_suspend` calls this + instead, so the wrapped awaitable's `await_ready` runs inside + `await_suspend` rather than at the start of the `co_await`. + + This avoids a second MSVC ARM64 release defect. When the wrapped + `await_ready` is evaluated at the start of the `co_await`, and + performs an atomic read-modify-write (as + `async_waker::wait_awaiter` does to consume a latched wakeup), a + `try` region spanning the `co_await` can lose its handler if a + call precedes the `try`. This happens even with the workaround + above in place. Observed on 19.51; not on 19.44. A plain or + atomic load in `await_ready` was not affected. The regression + tests are `testCatchAfterRmwAwaitReady` and + `testCatchAroundWakerWait` in test/unit/task.cpp, and + `testCatchAfterRmwAwaitReady` and + `testCatchStopAfterRmwAwaitReady` in test/unit/quitter.cpp. + + The awaiting coroutine continues without suspending when the + wrapped awaitable is ready, or when its `await_suspend` returns + `false`. When the wrapped `await_suspend` returns a handle, that + handle is resumed through `symmetric_transfer` and the awaiting + coroutine stays suspended. Returning `false` instead of resuming + inline keeps immediate completions from growing the stack. + + @param a The wrapped awaitable. + + @param h The coroutine performing the `co_await`. + + @param env The execution environment to pass to the wrapped + awaitable's `await_suspend`. + + @return `false` to continue without suspending, otherwise `true`. +*/ +template +bool +await_ready_then_suspend( + Awaitable& a, + std::coroutine_handle<> h, + await_context const* env) noexcept +{ + // Called exactly once: await_ready may consume state. + if(a.await_ready()) + return false; + using R = decltype(a.await_suspend(h, env)); + if constexpr (std::is_same_v) + { + return a.await_suspend(h, env); + } + else if constexpr (std::is_void_v) + { + a.await_suspend(h, env); + return true; + } + else + { + symmetric_transfer(a.await_suspend(h, env)); + return true; + } +} +#endif + // Helper to normalize await_suspend return types to std::coroutine_handle<> template std::coroutine_handle<> call_await_suspend( diff --git a/include/boost/capy/detail/config.hpp b/include/boost/capy/detail/config.hpp index 83b970396..95461f03c 100644 --- a/include/boost/capy/detail/config.hpp +++ b/include/boost/capy/detail/config.hpp @@ -77,8 +77,14 @@ - MSVC 19.34 through 19.44, every target: the returned handle is stored on the coroutine frame. - - MSVC 19.50 and later, ARM64 and ARM64EC, release builds: a - try region spanning the suspend point loses its handler. + - MSVC ARM64 and ARM64EC, release builds: a try region spanning + the suspend point loses its handler. Reproduced on 19.44 and + 19.51; the first clause already covers 19.44. + + Where it is on, task's and quitter's transform_awaiter also defer + the wrapped await_ready into await_suspend, through + detail::await_ready_then_suspend. That works around a related + ARM64 release defect the void-returning transfer does not cover. Not a configuration option: translation units that disagree on it get different definitions of symmetric_transfer. To check whether diff --git a/include/boost/capy/quitter.hpp b/include/boost/capy/quitter.hpp index 824a481ea..251a1aeea 100644 --- a/include/boost/capy/quitter.hpp +++ b/include/boost/capy/quitter.hpp @@ -351,12 +351,22 @@ struct [[nodiscard]] BOOST_CAPY_CORO_AWAIT_ELIDABLE arrives before an already-complete operation is observed by @ref await_resume, which runs in either case. + Where `BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND` is on, + this always returns `false`, and @ref await_suspend asks + the wrapped awaitable instead. See + `detail::await_ready_then_suspend`. + @return The wrapped awaitable's own `await_ready` result: - `true` if no suspension is needed. + `true` if no suspension is needed. Always `false` where + the workaround is on. */ bool await_ready() noexcept { +#if BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND + return false; +#else return a_.await_ready(); +#endif } /** Restore the frame allocator, check for stop, then resume the @@ -407,16 +417,24 @@ struct [[nodiscard]] BOOST_CAPY_CORO_AWAIT_ELIDABLE @return Whatever the wrapped awaitable's `await_suspend` returns. When that is a `std::coroutine_handle<>`, the handle is routed through `detail::symmetric_transfer`. - On MSVC that helper resumes the handle on the current - stack, and this function returns `void`, so the awaiting - coroutine suspends unconditionally. On every other - compiler the handle is returned unchanged for symmetric - transfer. + Where `BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND` is on, + this function instead returns `bool` from + `detail::await_ready_then_suspend`. It first asks the + wrapped awaitable's `await_ready`. When that reports + ready, it returns `false` to continue without + suspending. Otherwise it resumes any returned handle on + the current stack and returns `true`. Where the + workaround is off, a returned handle is passed back + unchanged for symmetric transfer. */ template auto await_suspend( std::coroutine_handle h) noexcept { +#if BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND + return detail::await_ready_then_suspend( + a_, h, p_->environment()); +#else using R = decltype( a_.await_suspend(h, p_->environment())); if constexpr (std::is_same_v< @@ -426,6 +444,7 @@ struct [[nodiscard]] BOOST_CAPY_CORO_AWAIT_ELIDABLE else return a_.await_suspend( h, p_->environment()); +#endif } }; diff --git a/include/boost/capy/task.hpp b/include/boost/capy/task.hpp index 67f9a8f21..57d0fedef 100644 --- a/include/boost/capy/task.hpp +++ b/include/boost/capy/task.hpp @@ -272,12 +272,22 @@ struct [[nodiscard]] BOOST_CAPY_CORO_AWAIT_ELIDABLE /** Report whether the wrapped awaitable is already complete. + Where `BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND` is on, + this always returns `false`, and @ref await_suspend asks + the wrapped awaitable instead. See + `detail::await_ready_then_suspend`. + @return The wrapped awaitable's own `await_ready` result: - `true` if no suspension is needed. + `true` if no suspension is needed. Always `false` where + the workaround is on. */ bool await_ready() noexcept { +#if BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND + return false; +#else return a_.await_ready(); +#endif } /** Restore the frame allocator, then resume the wrapped @@ -313,20 +323,28 @@ struct [[nodiscard]] BOOST_CAPY_CORO_AWAIT_ELIDABLE @return Whatever the wrapped awaitable's `await_suspend` returns. When that is a `std::coroutine_handle<>`, the handle is routed through `detail::symmetric_transfer`. - On MSVC that helper resumes the handle on the current - stack, and this function returns `void`, so the awaiting - coroutine suspends unconditionally. On every other - compiler the handle is returned unchanged for symmetric - transfer. + Where `BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND` is on, + this function instead returns `bool` from + `detail::await_ready_then_suspend`. It first asks the + wrapped awaitable's `await_ready`. When that reports + ready, it returns `false` to continue without + suspending. Otherwise it resumes any returned handle on + the current stack and returns `true`. Where the + workaround is off, a returned handle is passed back + unchanged for symmetric transfer. */ template auto await_suspend(std::coroutine_handle h) noexcept { +#if BOOST_CAPY_SYMMETRIC_TRANSFER_WORKAROUND + return detail::await_ready_then_suspend(a_, h, p_->environment()); +#else using R = decltype(a_.await_suspend(h, p_->environment())); if constexpr (std::is_same_v>) return detail::symmetric_transfer(a_.await_suspend(h, p_->environment())); else return a_.await_suspend(h, p_->environment()); +#endif } }; diff --git a/test/unit/quitter.cpp b/test/unit/quitter.cpp index 8c83c76b2..904b9f12e 100644 --- a/test/unit/quitter.cpp +++ b/test/unit/quitter.cpp @@ -866,6 +866,138 @@ struct quitter_test BOOST_TEST(!reached_end); } + //---------------------------------------------------------- + // 15. Catch around a co_await whose await_ready does an atomic + // read-modify-write (MSVC 14.51 ARM64 release regression) + // + // quitter's transform_awaiter gets the same safeguard as + // task's: where the symmetric-transfer workaround is on, the + // wrapped await_ready is asked from inside await_suspend. See + // testCatchAfterRmwAwaitReady in test/unit/task.cpp. Both + // tests below pass on every other build with or without it. + //---------------------------------------------------------- + + // An opaque call before the try; the defect needs one there. + static BOOST_CAPY_NOINLINE void + call_before_try(bool& marker) + { + marker = true; + } + + // Parks; await_ready performs a compare-exchange that never + // succeeds here, as async_waker's does. await_resume throws when + // throw_on_resume is set. + struct rmw_ready_awaitable + { + std::coroutine_handle<>& slot; + std::atomic& state; + bool throw_on_resume; + + bool await_ready() noexcept + { + int expected = 1; + return state.compare_exchange_strong( + expected, 0, + std::memory_order_acq_rel, + std::memory_order_acquire); + } + + std::coroutine_handle<> + await_suspend( + std::coroutine_handle<> h, await_context const*) noexcept + { + slot = h; + return std::noop_coroutine(); + } + + void await_resume() const + { + if(throw_on_resume) + throw test_exception("rmw ready"); + } + }; + + static quitter<> + rmw_ready_quitter( + std::coroutine_handle<>& slot, + std::atomic& state, + bool throw_on_resume, + bool& marker, + bool& caught) + { + call_before_try(marker); + try + { + co_await rmw_ready_awaitable{slot, state, throw_on_resume}; + } + catch(...) + { + caught = true; + } + } + + // The awaitable's own await_resume throws. + void + testCatchAfterRmwAwaitReady() + { + int dispatch_count = 0; + test_executor ex(dispatch_count); + + std::coroutine_handle<> parked; + std::atomic state{0}; + bool marker = false; + bool caught = false; + bool escaped = false; + bool completed = false; + + run_async(ex, + [&]() { completed = true; }, + [&](std::exception_ptr) { escaped = true; })( + rmw_ready_quitter(parked, state, true, marker, caught)); + + BOOST_TEST(static_cast(parked)); + BOOST_TEST(marker); + if(parked) + parked.resume(); + + BOOST_TEST(caught); + BOOST_TEST(!escaped); + BOOST_TEST(completed); + } + + // A stop request while parked: quitter's await_resume throws its + // stop sentinel, which the catch(...) in the body takes, as in + // the documentation's catch(...) pitfall. + void + testCatchStopAfterRmwAwaitReady() + { + int dispatch_count = 0; + test_executor ex(dispatch_count); + + std::stop_source source; + std::coroutine_handle<> parked; + std::atomic state{0}; + bool marker = false; + bool caught = false; + bool escaped = false; + bool completed = false; + + run_async(ex, source.get_token(), + [&]() { completed = true; }, + [&](std::exception_ptr) { escaped = true; })( + rmw_ready_quitter(parked, state, false, marker, caught)); + + BOOST_TEST(static_cast(parked)); + BOOST_TEST(marker); + source.request_stop(); + if(parked) + parked.resume(); + + BOOST_TEST(caught); + BOOST_TEST(!escaped); + BOOST_TEST(completed); + } + //---------------------------------------------------------- // 13. Echo server with shutdown // @@ -989,6 +1121,8 @@ struct quitter_test testWhenAllWithStop(); testWhenAnyWithStop(); testWakerCancellation(); + testCatchAfterRmwAwaitReady(); + testCatchStopAfterRmwAwaitReady(); testEchoWithShutdown(); } }; diff --git a/test/unit/task.cpp b/test/unit/task.cpp index f26537bcc..003ff9c8c 100644 --- a/test/unit/task.cpp +++ b/test/unit/task.cpp @@ -1,5 +1,6 @@ // // Copyright (c) 2025 Vinnie Falco (vinnie.falco@gmail.com) +// Copyright (c) 2026 Michael Vandeberg // // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -11,6 +12,7 @@ #include #include +#include #include #include #include @@ -1631,6 +1633,50 @@ struct task_test int await_resume() noexcept { return 7; } }; + // Not ready, and await_suspend returns void: the coroutine stays + // suspended until the test resumes it. Where the symmetric-transfer + // workaround is on, this is the void branch of + // detail::await_ready_then_suspend. + struct void_suspend_awaitable + { + std::coroutine_handle<>& slot; + + bool await_ready() const noexcept { return false; } + + void await_suspend( + std::coroutine_handle<> h, await_context const*) noexcept + { + slot = h; + } + + int await_resume() const noexcept { return 7; } + }; + + static task + void_suspend_session(std::coroutine_handle<>& slot) + { + co_return co_await void_suspend_awaitable{slot}; + } + + void + testVoidSuspendAwaitable() + { + int dispatch_count = 0; + test_executor ex(dispatch_count); + + std::coroutine_handle<> parked; + int result = 0; + + run_async(ex, [&](int v) { result = v; })( + void_suspend_session(parked)); + + BOOST_TEST(static_cast(parked)); + BOOST_TEST_EQ(result, 0); + if(parked) + parked.resume(); + BOOST_TEST_EQ(result, 7); + } + void testBoolSuspendAwaitable() { @@ -1732,6 +1778,155 @@ struct task_test BOOST_TEST(completed); } + // An opaque call placed before the try in the two tests below. + // The MSVC ARM64 defect they cover needs a call there. + static BOOST_CAPY_NOINLINE void + call_before_try(bool& marker) + { + marker = true; + } + + // Parks like parking_awaitable, but its await_ready performs an + // atomic read-modify-write that never succeeds here, as + // async_waker's does to consume a latched wakeup. await_resume + // throws. + struct rmw_ready_awaitable + { + std::coroutine_handle<>& slot; + std::atomic& state; + + bool await_ready() noexcept + { + int expected = 1; + return state.compare_exchange_strong( + expected, 0, + std::memory_order_acq_rel, + std::memory_order_acquire); + } + + std::coroutine_handle<> + await_suspend( + std::coroutine_handle<> h, await_context const*) noexcept + { + slot = h; + return std::noop_coroutine(); + } + + void await_resume() const + { + throw_test_exception("rmw ready"); + } + }; + + static task + rmw_ready_session( + std::coroutine_handle<>& slot, + std::atomic& state, + bool& marker, + bool& caught) + { + call_before_try(marker); + try + { + co_await rmw_ready_awaitable{slot, state}; + } + catch(test_exception const&) + { + caught = true; + } + } + + // Regression test for a second MSVC 14.51 ARM64 release defect, + // which the void-returning symmetric_transfer does not cover. + // + // With a call before the try, and an awaiter whose await_ready + // performs an atomic read-modify-write, the throw from + // await_resume was not caught by the catch beside it. It + // escaped to run_async's error handler. A plain or atomic load + // in await_ready did not trigger it. transform_awaiter avoids it + // by asking the wrapped await_ready from inside await_suspend + // (detail::await_ready_then_suspend). + // + // As with testCatchAfterDeferredResume, the escape is checked + // through the error handler so it cannot terminate the binary. + void + testCatchAfterRmwAwaitReady() + { + int dispatch_count = 0; + test_executor ex(dispatch_count); + + std::coroutine_handle<> parked; + std::atomic state{0}; + bool marker = false; + bool caught = false; + bool escaped = false; + bool completed = false; + + run_async(ex, + [&]() { completed = true; }, + [&](std::exception_ptr) { escaped = true; })( + rmw_ready_session(parked, state, marker, caught)); + + BOOST_TEST(static_cast(parked)); + BOOST_TEST(marker); + if(parked) + parked.resume(); + + BOOST_TEST(caught); + BOOST_TEST(!escaped); + BOOST_TEST(completed); + } + + static task + waker_session( + async_waker& w, + bool& marker, + bool& caught) + { + call_before_try(marker); + try + { + auto [ec] = co_await w.wait(); + (void)ec; + throw_test_exception("woken"); + } + catch(test_exception const&) + { + caught = true; + } + } + + // The same defect through async_waker, the shape that first + // exposed it: a call before the try, then a wait. On MSVC 14.51 + // ARM64 release the throw after a successful wait escaped the + // catch. test_executor posts inline, so wake() resumes the + // session on this stack. + void + testCatchAroundWakerWait() + { + int dispatch_count = 0; + test_executor ex(dispatch_count); + + async_waker w; + bool marker = false; + bool caught = false; + bool escaped = false; + bool completed = false; + + run_async(ex, + [&]() { completed = true; }, + [&](std::exception_ptr) { escaped = true; })( + waker_session(w, marker, caught)); + + BOOST_TEST(marker); + BOOST_TEST(!caught); + w.wake(); + + BOOST_TEST(caught); + BOOST_TEST(!escaped); + BOOST_TEST(completed); + } + void run() { @@ -1742,7 +1937,10 @@ struct task_test testDestroyUnstartedOrFinishedTaskIsFine(); #endif testCatchAfterDeferredResume(); + testCatchAfterRmwAwaitReady(); + testCatchAroundWakerWait(); testBoolSuspendAwaitable(); + testVoidSuspendAwaitable(); testReturnValue(); testException(); testTaskAwaitsTask();