Skip to content

core, wire, net, qt: a hello-negotiated cancel envelope, honoured by RemoteServer and sent by both client backends (#864, #865) - #867

Merged
Yaraslaut merged 2 commits into
masterfrom
lane/wire-cancel
Oct 4, 2026
Merged

Yaraslaut merged 2 commits into
masterfrom
lane/wire-cancel

Conversation

@Yaraslaut

@Yaraslaut Yaraslaut commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Closes #864
Closes #865

Two commits, one per ticket, server side first; either can be dropped and the branch force-pushed without redoing the other.

#864 — server side (core, wire)

  • New "cancel" kind. The target travels in a new Envelope::cancelCallId; the cancel has its own callId. Were the target in callId, the server's ok to the cancel would be matched by the client against the still-pending execute and settle it with an empty result.

  • Negotiated through hello: ProtocolRange gains capabilities (the server lists "cancel"), with wire::helloAdvertises(reply, wire::kCapabilityCancel) to read it. Both members are additive, so this is a capability and not a kProtocolVersion bump. A bump would have refused every v1 client under the default range. A legacy server never receives a cancel.

  • RemoteServer files every admitted Task execute that arrived on a connection scope, together with the stop source and the facts its admission decided on: the verified principal, model and action type, instance, and owner. A cancel is handled in this order:

    1. It is stamped with stampVerifiedPrincipal.
    2. The call is looked up under the connection the cancel arrived on, plus cancelCallId.
    3. The cancel's verified principal must equal the execute's.
    4. The cancel must pass the execute's own authorize and authorizeInstance.

    Only then is request_stop() called. Every outcome (stopped, unknown, finished, foreign connection, foreign principal, refused, unscoped) gets the same ok, so a cancel cannot probe for calls.

  • Specs: wire.md ("Cancelling a call"), backend.md (envelope table, execute flow), security.md ("On cancel").

Security tests (tests/test_remote_cancel.cpp)

  • A blocking Task handler gets the stop after cancel. The execute replies err, and the cancel's reply carries its own callId.
  • A cancel from another connection, with the same callId and the same principal, has no effect: the call completes ok with its value.
  • Two principals share an unowned instance over one connection, so only the recorded principal tells them apart. A cancel with bob's token claiming "alice" has no effect, and neither does a tokenless "alice" claim. Control case: alice's token with a "mallory" claim does stop the call.
  • On an owned instance, only the owner's cancel stops the call.
  • A cancel for an unknown callId, a finished call, or an unscoped (cid 0) call is a no-op ok.

Mutation checks (real output, from a probe binary built from test_remote_cancel.cpp)

  • M1, mapping removed (stop->request_stop() → no-op):
    RemoteServer: on an owned instance only the owner's cancel stops the call ... FAILED: REQUIRE( call.await() ) with expansion: false
    RemoteServer: a cancel stops the Task handler of the call it names ... FAILED: REQUIRE( call.await() ) with expansion: false
    RemoteServer: a cancel keys on the verified principal ... control: alice's own token stops it ... FAILED: REQUIRE( call.await() ) with expansion: false
    test cases:  8 |  5 passed | 3 failed
    
  • M2, principal ownership check removed:
    RemoteServer: a cancel keys on the verified principal, not the claimed one
      bob's token claiming to be alice has no effect
    test_remote_cancel.cpp:278: FAILED: CHECK( reply.kind == "ok" ) with expansion: "err" == "ok"
    test_remote_cancel.cpp:279: FAILED: CHECK( probe().stopped.load() == 0 ) with expansion: 1 == 0
    test cases:  8 |  7 passed | 1 failed
    
  • M3, connection scoping removed (lookup by callId across connections):
    (a cancel from another connection has no effect on the call)
    test_remote_cancel.cpp:254: FAILED: CHECK( reply.kind == "ok" ) with expansion: "err" == "ok"
    test_remote_cancel.cpp:255: FAILED: CHECK( reply.body == "41" ) with expansion: "" == "41"
    test_remote_cancel.cpp:256: FAILED: CHECK( probe().stopped.load() == 0 ) with expansion: 1 == 0
    test cases:  8 |  7 passed | 1 failed
    
  • M4, stampVerifiedPrincipal removed from the cancel branch: "bob's token claiming to be alice" goes red (lines 278 and 279), and so does the control (295). test cases: 8 | 7 passed | 1 failed.

#865 — client side (net, qt)

  • Both backends learn the capability from negotiateProtocolVersion, which is opt-in. SocketBackend gains that verb, mirroring Qt's. After every reconnect they re-send hello, and on every disconnect they forget the capability. They send cancel only when it was advertised. The cancel is fire-and-forget, under a fresh callId that is filed nowhere.
  • Triggers:
    • Stop requested (an execute deadline, or any holder of ActionCall::stopSource). A StopCallback is registered once the call has its id and its frame is queued. It only posts requestCancel(callId) to the owner: the loop for SocketBackend, the Qt thread for Qt. The owner sends the cancel only if the call is still waiting for its reply.
    • cancelPending (~Bridge, switchBackend). A cancel goes out for every drained execute.
  • The cancel carries the execute's own session, because the server honours it only under the execute's verified principal.
  • Teardown fix found by the measurement. SocketBackend::close cleared the outbox, and ~QtWebSocketBackend aborted the socket with the frames still unsent. Either way, the cancels ~Bridge's cancelPending had just queued were dropped, and the ~Bridge tests failed on both backends. SocketBackend now uses close-after-flush, still bounded by sendTimeout. Qt now calls _socket.flush() before abort(), which does not block.
  • Ordering note in the spec, as asked in the ticket. SocketBackend::cancelPending is posted to the loop (G0 on return), and that is acceptable:
    • A cancel can never overtake its execute. Both go through the same FIFO loop and the same connection, and the server processes one connection in order.
    • Only the server handler's stop is delayed, and only by non-blocking loop work.
    • The caller's own completion is settled in that same loop task.
  • scripts/branch_partial_allowlist.json: the socket_backend.hpp default: hint moves from line 797 to 966. The source text is unchanged and unique.

Tests (real loopback)

  • tests/net/test_socket_backend.cpp [cancel], with SocketServer and SocketBackend:
    • A client deadline makes the server's Task handler observe the stop.
    • Destroying the Bridge does the same.
    • With no negotiation, the deadline leaves the handler running, and it completes with its item.
    • Against a legacy fake server, the backend never sends a cancel: the next frame after a stop and a cancelPending is the marker deregister.
  • tests/qt/test_qt_websocket.cpp [cancel], against a real QtWebSocketServer: the deadline test and the ~Bridge test. Qt 6 (Homebrew) was available locally, and both were run.

Mutation checks (real output)

  • SocketBackend, stop-triggered send removed:
    SocketBackend: a client deadline stops the server's Task handler
    test_socket_backend.cpp:2558: FAILED: CHECK( scParkProbe().stopped.load() == 1 ) with expansion: 0 == 1
    test cases:  4 |  3 passed | 1 failed
    
  • SocketBackend, cancelPending send removed:
    SocketBackend: destroying the bridge cancels its calls on the server
    test_socket_backend.cpp:2577: FAILED: CHECK( scParkProbe().stopped.load() == 1 ) with expansion: 0 == 1
    
  • SocketBackend, capability gate removed (sends without an advertisement):
    SocketBackend: never sends cancel to a server that did not advertise it
    test_socket_backend.cpp:2637: FAILED: CHECK( fake.receiveEnvelope().kind == "deregister" ) with expansion: "cancel" == "deregister"
    SocketBackend: without a negotiated cancel, a deadline leaves the server's handler running
    test_socket_backend.cpp:2600: FAILED: CHECK( scParkProbe().stopped.load() == 0 ) with expansion: 1 == 0
    
  • SocketBackend with the old close (outbox discarded), measured before the fix: the ~Bridge test failed, 0 == 1.
  • Qt, stop-triggered send removed:
    morph::qt::QtWebSocketBackend: a client deadline stops the server's Task
    test_qt_websocket.cpp:2857: FAILED: CHECK( wsParkProbe().stopped.load() == 1 ) with expansion: 0 == 1
    
  • Qt with no flush before abort, measured before the fix: the ~Bridge test failed, 0 == 1.

Qt stop path: no templated invokeMethod (clang-tidy-diff fix)

CI's clang-tidy 22 with Qt 6.8.1 reported clang-analyzer-cplusplus.NewDeleteLeaks at QtCore/qobjectdefs.h:624. The path ran through the functor overload QMetaObject::invokeMethod(context, lambda, Qt::QueuedConnection) in CancelOnStop.

The leak report is a Qt false positive:

  • In 6.8.1, invokeMethodCallableHelper puts its NOLINTNEXTLINE on the new QCallableObject line, but the analyzer reports at the return invokeMethodImpl(...) two lines later.
  • A NOLINT on our line cannot suppress a diagnostic that is located in Qt's header.

So the functor call is gone from the changed code instead of suppressed:

  • CancelOnStop sets a per-call std::shared_ptr<std::atomic<bool>> flag.
  • It then calls the member-name overload QMetaObject::invokeMethod(&_cancelWake, "start", Qt::QueuedConnection). In 6.8.1 that reaches invokeMethodImpl(QObject*, const char*, ...) with no new; I read qobjectdefs.h at v6.8.1, lines 372–389.
  • _cancelWake is a zero-interval single-shot QTimer. Its timeout is connected once, in the constructor, to drainCancels(), the same lambda-connect pattern as the constructor's existing connects.
  • The drain runs on the socket's thread and sends a cancel for each flagged pending call. _pending stays owner-only, with no lock.

Why this shape: it is the smallest one that allocates no callable in changed code. A postEvent(new QEvent) alternative would have needed an owning-memory NOLINT.

Not reproduced locally. Homebrew Qt here is 6.11.1, whose headers already widen the suppression, so local clang-tidy 23 never showed the finding. It shows no NewDeleteLeaks before or after this change. The claim that this fixes CI rests on the new code making no templated invokeMethod/singleShot call.

Re-measured after the change:

  • Qt [cancel] (deadline and ~Bridge): green, 25 of 25 repeats.
  • Full morph_qt_tests: 85 of 85 passed.
  • Mutation, drain's send removed: test_qt_websocket.cpp:2857: FAILED: CHECK( wsParkProbe().stopped.load() == 1 ) with expansion: 0 == 1.
  • Mutation, wake removed (flag set, timer never started): same failure, 0 == 1.

WsParkServer park is now const in both Qt tests, the other two findings.

Cancellation-policy rows to move in docs/spec/concurrency_and_lifetimes.md once both PRs land

That file belongs to the other lane, so this PR does not touch it. The proposed rows are in backend.md under "Cancellation-policy rows for the remote backends":

  • SocketBackend::cancelPending: Work becomes "a Task handler on a server that advertised cancel is asked to stop; otherwise the server keeps executing". The work column is measured (net suite, via ~Bridge). The G-level is unchanged and still read.
  • QtWebSocketBackend::cancelPending: the same, measured in the Qt suite via ~Bridge. The G-level is still read.
  • Bridge::setExecuteDeadline: add that over either remote backend, the deadline's stop reaches the server's Task handler through a cancel (measured, net and Qt).
  • "What no verb does today": remove the bullet "No cancel crosses the wire".

Verification

  • morph_tests: 1752 test cases. 1751 passed and 1 failed as expected; that is the suite's pre-existing [!shouldfail] case.
  • morph_net_tests: all passed, 214 cases.
  • morph_qt_tests: all passed, 85 cases.
  • morph_net_qt_interop_tests: all passed.
  • Repeats:
    • morph_tests [cancel]: 475 runs with one failure, which happened in the first batch of 25. That failure's output was not captured: it ran while another lane was compiling. The next 450 runs had none. Weak evidence of a timing flake (2 s waitUntil budgets under load), not reproduced.
    • net [cancel]: 50 runs, 0 failures.
    • Qt [cancel]: 50 runs, 0 failures.
  • clang-format has been run on every changed file.
  • clang-tidy was run locally with LLVM 23 (CI uses 22), restricted to changed lines. All findings are fixed except two that are specific to the newer version: readability-trailing-comma and lifetime-safety-*. Existing code on master has the same patterns, and CI 22 passes them.
  • The Doxygen doc target builds with no warnings.

Review notes (done inline)

  • The server allocates a stop source only for Task executes on scoped connections (or when executeTimeout is set). An ordinary handler's execute is unchanged.
  • The cancellable map is touched only on the server strand. An entry is removed by callFinished, which is posted with the reply, or by the cancel that stops it. A reused callId on one connection replaces the entry, and callFinished erases only its own entry, identified by its stop source.
  • A stop callback only posts, so destroying it on the owner (it waits for an in-flight callback) cannot deadlock.
  • A Qt cancel that is queued but not yet run either dies with the socket, or meets _shuttingDown during the destructor's processEvents.

Not verified

  • ASan, TSan, valgrind and coverage were not run locally. The branch_partial_allowlist hint was updated by hand, and check_branch_coverage.py was not run.
  • Linux, Windows and WASM builds were not run. Only macOS/arm64 with Apple clang was tested.
  • The G-levels of the remote backends' cancel verbs were not re-measured.
  • Not addressed: RemoteServer::closeConnection still does not stop a dropped connection's running Task handlers. That is a possible follow-up and is not filed here.

🤖 Generated with Claude Code

Yaraslaut and others added 2 commits October 4, 2026 12:29
… onto the run's stop source (#864)

A `cancel` names the execute it stops by `cancelCallId` and carries its own
`callId`, so the server's `ok` to it can never be matched against the still
pending execute. The server advertises the kind in its `hello` reply
(`ProtocolRange::capabilities`, "cancel"); a client sends it only after such a
reply, so a server that predates it never sees one. Both members are additive,
so this is a capability, not a kProtocolVersion bump.

RemoteServer files each admitted Task execute that arrived on a connection
scope, with what its admission decided on: the verified principal, the model
and action types, the instance and its owner. A cancel is stamped
(stampVerifiedPrincipal), looked up only under the connection it arrived on,
and honoured only when its verified principal is the execute's and it passes
the execute's own authorize and authorizeInstance. Every outcome — stopped,
unknown, finished, another connection's, another principal's, refused,
unscoped — answers the same `ok`, so a cancel cannot probe for calls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…'s stop is requested (#865)

Both backends learn from the opt-in negotiateProtocolVersion whether the
server honours "cancel" (SocketBackend gains the verb), re-send hello after
every reconnect and forget the capability on every disconnect. Only then do
they send `cancel {cancelCallId}`, fire-and-forget under a fresh callId:

- when a call's stop is requested (an execute deadline, or any holder of
  ActionCall::stopSource) — a stop callback registered once the call has its
  id and its frame is queued, which hands the cancel to the backend's owner;
- for every execute cancelPending sweeps (~Bridge, switchBackend).

The cancel carries the execute's own session, since the server honours it
only under the execute's verified principal.

Teardown lets the cancels out: SocketBackend's close is a close-after-flush on
the loop, and ~QtWebSocketBackend flushes before its abort. Both discarded the
frames ~Bridge's cancelPending had just queued.

SocketBackend::cancelPending is posted to the I/O loop (G0 on return); the
backend spec states why a cancel queued behind earlier loop work is
acceptable: it can never overtake its execute, the delay is only to the
server's handler, and the caller's own completion is settled in that task.

Tests over real loopbacks (SocketServer + SocketBackend, QtWebSocketServer +
QtWebSocketBackend): a client deadline, and destroying the Bridge, each make
the server's Task handler observe the stop; a client that did not negotiate,
or negotiated with a legacy server, sends none.

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 89.18919% with 16 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
include/morph/net/socket_backend.hpp 83.56% 4 Missing and 8 partials ⚠️
include/morph/core/remote.hpp 93.22% 1 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant