Skip to content

core: client-side stop plumbing: a stopped co_await stops its call; execute(action, StopToken) - #866

Merged
Yaraslaut merged 3 commits into
masterfrom
lane/stop-plumbing
Oct 4, 2026
Merged

Yaraslaut merged 3 commits into
masterfrom
lane/stop-plumbing

Conversation

@Yaraslaut

@Yaraslaut Yaraslaut commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Client-side stop plumbing: two tickets, one commit each.

Closes #862
Closes #863

#862: a stopped co_await stops the call (be7a14f)

  • CompletionAwaiter::attach links the awaiter's scope to the awaited call's stop source with CompletionState::linkStop, the same way a scope-gated continuation does.
  • linkStop was not the whole fix, as the issue suspected and as noted on core: a stopped co_await ends the call, not only the await (link CompletionAwaiter to the call's stop) #862. A stop that lands before attach runs never reaches the call: a token already stopped at co_await, or a stop that races the registration of the stop callback. In both cases stopUnattached relays the stop to the call on the completion's executor.
  • Spec: a new measured G2 row in the cancellation policy, and the "a co_await withdraws the await, not the call" bullet is removed. The Cancellation section of coroutines.md now says what the stop does to the call.

#863: BridgeHandler::execute(action, core::async::StopToken) (2730860)

  • A stop before the call settles rejects the completion with OperationCancelled. The rejection is posted from the thread that requested the stop. The stop is also requested on the call's StopSource, so a Task handler on a LocalBackend sees it. A token already stopped rejects the call without dispatching it.
  • The mechanism is CompletionState::linkCancel, held in stopLinks with the scope links and released by deliver(). It settles first and then stops, so OperationCancelled wins over the outcome the stopped handler produces. Like the execute deadline, it settles the state and not the sink, so the call stays counted as pending until the backend's own reply arrives.
  • Spec: a new G2 row. It says plainly that on a remote backend the call is only abandoned: no cancel crosses the wire until core, wire: a hello-negotiated cancel envelope that RemoteServer maps onto the run's StopSource #864/net, qt: SocketBackend and QtWebSocketBackend send cancel when a call's stop is requested #865, so the server's handler runs to its end and its reply is dropped. That case is measured with SimulatedRemoteBackend. completion.md and bridge.md no longer claim that no work cancellation exists.
  • API shape: an overload, not an option struct. The existing caller-facing verbs are a set of one-argument overloads, execute(Action) and executeJson(...). A trailing StopToken is the core-cpp and std::jthread convention, adds one overload without touching any existing call, and leaves room for an options struct later if per-call options grow. The verb is on BridgeHandler because that is the caller-facing surface the spec documents. Bridge::executeVia and its variants take a HandlerBinding and are the handler's plumbing. executeJson gets no token variant here.

Mutation checks (real output)

#862: with state.linkStop(token.stopToken()) removed from attach:

A stopped co_await is G2 for the await, and stops the Task handler it was
waiting on
tests/test_cancellation_policy.cpp:588: FAILED:
  REQUIRE( pumpUntil(owner, [] { return sleeper().cancelled.load() == 1; }) )
with expansion:
  false
test cases: 2 | 1 passed | 1 failed

The case with the token already stopped stayed green under that mutation, because it goes through stopUnattached. That relay was mutated separately, in the same build as the #863 mutation below, and its test went red (:727, same sleeper().cancelled assertion).

#863: with the token ignored (execute(action, stop) forwards to execute(action)):

execute with a stop token is G2 when stopped, and stops the Task handler running the call
tests/test_cancellation_policy.cpp:482: FAILED:
  REQUIRE( pumpUntil(owner, [&] { return outcome.err == 1; }) )
execute with a stop token stopped from another thread settles the call; a synchronous action runs on
tests/test_cancellation_policy.cpp:511: FAILED:  (same assertion)
execute with a stop token already stopped rejects the call without dispatching it
tests/test_cancellation_policy.cpp:536: FAILED:  (same assertion)
execute with a stop token on SimulatedRemoteBackend abandons the call; the server's handler runs on
tests/test_cancellation_policy.cpp:562: FAILED:  (same assertion)
test cases:  21 |  16 passed | 5 failed

The settle assertion is a REQUIRE, so it aborts the case before the handler-stop assertion runs. With the token ignored, nothing could stop the handler anyway.

Review, done inline

  • Threads. CancelRelay runs on whichever thread requests the stop. setException is already safe from any thread, as the deadline relies on. It reads the stop source through a weak_ptr captured on the owner, so it never reads stopSource off the owner.
  • Lifetimes. The relay holds the state and the stop source weakly, so a link that outlives its call keeps nothing alive. It cannot form a cycle through stopLinks.
  • Deliver racing a stop. If deliver() drops a link while its callback is running on another thread, the drop waits for that callback, as the existing scope links already do. The callback never blocks on the owner, so this cannot deadlock.
  • Await after a normal settle. The awaiter's destructor still stops its scope. deliver() keeps the links alive until it returns, so a coroutine that resumes inline and finishes there can request stop on a call that has already settled. That is harmless, since the call has nothing left to stop, but it is visible in a trace.
  • Allowlist. One #include shifts bridge.hpp, so the branch_partial_allowlist.json hint moves from 1772 to 1773. All 13 entries' source lines were re-checked and match.

Verification

  • morph_tests, full run: test cases: 1750 | 1749 passed | 1 failed as expected.
  • [cancel-policy]: 25 runs, 25 passed, 0 failed.
  • clang-format 23.1 on the changed files.
  • clang-tidy-diff.py with local clang-tidy 23.1 (CI pins 22): two findings fixed (readability-trailing-comma, misc-const-correctness). One remains, misc-const-correctness on errCode inside the glaze expansion of BRIDGE_REGISTER_ACTION(CPModel, CPSleep, ...). The identical registration lines core: stamp the verified principal on deregister, release only held references; spec + tests for the cancellation policy (#855, #858, #846) #859 added passed CI's clang-tidy 22, so I expect 22 not to raise it. That is inferred, not run.
  • The Doxygen doc target exits 0 with no warnings.

Not verified:

  • clang-tidy 22 itself.
  • Linux, MSVC, sanitizer and Valgrind jobs.
  • MORPH_BUILD_NET and MORPH_BUILD_QT builds. Those remote backends carry no client-side stop source, so for them both verbs only abandon or withdraw.
  • The six ctest entries outside morph_tests (journal skew, client-only, canary), which need targets I did not build.

Separate commit: a test's stack-use-after-scope (not part of either ticket)

On the rebased run, Linux / clang-asan reported a stack-use-after-scope at tests/test_async_registration.cpp:556, in "Bridge::assignHandlerPrimary: a promotion reply after the handler or the bridge is gone is a no-op". The test declared each Outcome inside the scope it was destroying, but attached ungated then/onError handlers that capture it. Whether a late reply was delivered into the dead object depended on whether the pool's settle landed inside a 5 ms runFor window. The same test passed ASan on #867's run. Neither ticket's code is involved, since the test passes no StopToken. Fixed in place, as AGENTS.md asks for tests: both outcomes now live at test scope. The ASan failure was not reproduced locally. The fix holds by construction: the outcomes outlive every pump.

🤖 Generated with Claude Code

@Yaraslaut Yaraslaut added enhancement New feature or request area: core Subsystem: core labels Oct 4, 2026
Yaraslaut and others added 2 commits October 4, 2026 13:59
CompletionAwaiter now links its scope to the awaited call's stop source
through CompletionState::linkStop when it attaches, as a scope-gated
continuation already does. A stop that withdraws the await, or the frame
destroyed while suspended, therefore also asks the call to stop: a Task
handler on a LocalBackend sees OperationCancelled at its next stop-aware
await.

linkStop alone was not the whole fix. A stop that lands before the
handlers are attached (a token already stopped at co_await, or a stop
racing the stop-callback registration) never reaches attach(), so the
awaiter now relays that stop to the call on the completion's executor.

The cancellation policy gains a measured G2 row for a stopped co_await;
the coroutine spec's Cancellation section says what the stop now does to
the call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n) (#863)

A stop on the token before the call settles rejects the Completion with
core::async::OperationCancelled, posted from the thread that requested
the stop, and requests stop on the call's StopSource, so a Task handler
on a LocalBackend sees it. A synchronous handler runs to its end; on a
remote backend the call is only abandoned, since no cancel crosses the
wire yet. A token already stopped rejects the call without dispatching.

The link is CompletionState::linkCancel, held with the scope links in
stopLinks and released by deliver(). Like the execute deadline it settles
the state, not the sink, so the call stays counted pending until the
backend's own reply.

The cancellation policy gains a measured G2 row for the verb, stating
the remote limitation; completion.md and bridge.md no longer say that no
work cancellation exists.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
include/morph/core/detail/completion_awaiter.hpp 90.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

…deliver into them

"a promotion reply after the handler or the bridge is gone is a no-op"
declared each Outcome inside the scope it was destroying, but attached
ungated then/onError handlers that capture it. A reply settling inside a
later runFor() is delivered into it, so whether the test read a destroyed
stack object depended on whether the pool's settle landed inside the 5 ms
window. clang-asan caught it once as a stack-use-after-scope at
test_async_registration.cpp:556. Both outcomes now live at test scope.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Yaraslaut
Yaraslaut merged commit bc9c35d into master Oct 4, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: core Subsystem: core enhancement New feature or request

Projects

None yet

1 participant