core: SynchronousBackendAdapter::cancelPending settles the completion but the queued control call still registers on the wrapped backend

[Yaraslaut]

Summary

Found while fixing #619, and deliberately not folded into it: #619 is about
the completion the adapter produced never being settled by cancelPending;
this is about the work behind that completion still happening after the
cancellation. Fixing one does not fix the other, and they need different
mechanisms.

SynchronousBackendAdapter::dispatch (include/morph/core/backend.hpp) posts a
task to its private _control strand:

_control.post(kControlStrand, [shared, op = std::move(op)]() mutable {
    try {
        shared->resolve(op());
    } catch (...) {
        shared->reject(std::current_exception());
    }
});

After #619, cancelPending rejects shared. It does nothing to op. A task
that has not started yet still runs op() when it reaches the head of the
strand, which for a bind is inner->bindModelBlocking(request) — an actual
registerModelWithContext/registerModelShared/attachModel on the wrapped
backend. The resolve that follows is a no-op on an already-rejected state, so
the caller is told the bind was cancelled while the registration goes through
anyway.

Why it matters

cancelPending's two callers are ~Bridge (bridge.hpp) and
Bridge::switchBackend. In the ~Bridge case the instance is created on a
backend whose Bridge is gone, so nothing will ever deregisterModel it: it
is a leaked live instance for as long as the backend lives. In the
switchBackend case it is created on the outgoing backend after the swap.

Severity today is latent for the same reason #619's is: grep -rn SynchronousBackendAdapter finds no production call site on this revision, only
tests/test_backend_registration_surface.cpp and prose. morph#571, which
proposes making this surface the default path, is what would make it live.

Verification status

Reproduced, as a side observation of #619's own regression test, on the
laneCORE-batch-615-619-620 branch (base a8511aa6), GCC 16.2.1, Debug,
Linux. The test wraps a backend whose registerModelWithContext blocks until
released, cancels while it is blocked, then releases it. Real output from that
case, which passes:

All tests passed (92 assertions in 11 test cases)

The assertions that record this behaviour are, after the cancellation has
already been delivered as an error:

    inner->letGo();
    REQUIRE(morph::testing::waitUntil([&] {
        callerExec.runOnce();
        return inner->finished.load() == 1;
    }));

inner->finished is incremented by the wrapped backend's control call on its
way out, so == 1 is the observation: the registration completed after
cancelPending returned. The test asserts it because that is current
behaviour; it is the assertion that would have to change if this issue is
fixed.

What I did not verify: that any real backend leaks anything as a result. No
production call site exists to leak through, so the leak is inferred from
reading ~Bridge/switchBackend (neither deregisters an instance it never
learned the id of), not measured.

Sketch of a fix (not implemented)

Give each dispatched task a shared_ptr<std::atomic_bool> cancellation flag
alongside its promise, set every live one in cancelPending, and have the task
return without calling op() when it is set. That closes the "queued but not
started" window, which is all that can be closed: a call already inside
op() cannot be recalled, and SynchronousBackendAdapter has no way to
interrupt a blocking verb it does not implement.

What would change the verdict

• Close it if a maintainer decides the adapter is documented as best-effort
  here — that a cancelled bind may still register — in which case
  IBackend::cancelPending's contract wants that carve-out written down, since
  the wrapped backend's own cancelPending does not have it.
• Raise severity if morph#571 lands and this adapter becomes a production
  path, or if any caller is found that relies on cancelPending meaning "no
  further registration will reach the backend".

[Yaraslaut]

Effect of #571 on this issue: none, in either direction. Recorded because the lane that removed the four *Async twins was asked to say so if its work made this easier or harder to fix.

#571 (branch laneLADDER-batch-570-571, commit 085b401b) removes registerModelAsync, registerModelSharedAsync, attachModelAsync and assignPrimaryAsync from IBackend and from SynchronousBackendAdapter's forwarding block. It touches neither half of the mechanism this issue describes:

• The task body queued on _control is still inner->bindModelBlocking(std::move(request)) (and inner->assignPrimary(...) for the promote). None of the four removed verbs appeared in it, before or after.
• SynchronousBackendAdapter::cancelPending is unchanged by core: remove IBackend's four async twins and retire the prose threading contract #571 — it is still what core: SynchronousBackendAdapter::cancelPending does not cancel the completions the adapter itself produced #619 left: reject the adapter's own tracked promises, then forward to _inner. The queued-task-still-runs gap is the same statement about the same code.

What did change is the adapter's surface area: it now forwards only the synchronous verbs, because the one verb that could have carried a non-blocking path (bindModel) is the verb it reshapes. That makes the file smaller, not the fix different.

Verification status. Established by reading include/morph/core/backend.hpp on 085b401b and diffing the adapter against origin/master (c55ea5b7); I did not reproduce this issue's measured symptom and did not write a test for it. Nothing here should be read as evidence about whether the gap still reproduces — only that #571's diff does not reach it.

What would change the verdict: a fix for this issue that needs a per-request handle threaded from bindModel into the _control task would now have exactly one dispatch verb to thread it through instead of five, which is marginally simpler than it would have been before #568. That is the only sense in which the chain helped, and it is #568's doing rather than #571's.

[Yaraslaut]

Triage: valid — the premise holds on f7c231df, and the code already says so

#639 has landed, so this is now measurable against master rather than against a branch.

SynchronousBackendAdapter::cancelPending does what #619 asked — it rejects the completions the adapter itself produced, then forwards:

void cancelPending(const std::exception_ptr& exc) override {
    std::vector<std::weak_ptr<BindPromise>> snapshot;
    { std::scoped_lock const lock{_pendingMtx}; snapshot.swap(_pending); … }
    for (auto& weak : snapshot) { if (auto promise = weak.lock()) promise->reject(exc); }
    _inner->cancelPending(exc);
}

And the gap this issue describes is written into the header four lines above it (include/morph/core/backend.hpp:1164):

may still happen — morph#636, since stopping it needs the task to check before calling op(), not the promise to be settled after it.

So the defect is real, acknowledged in the shipped code, and correctly scoped: settling the promise cannot stop a task that has already been queued on _control; only the task testing a cancellation flag before it calls op() can. #639 was right to land the half it could and record the half it could not, rather than claiming the whole.

triage: valid, bug, area: core.

Verification status: read from the code, not reproduced. I did not write a test that queues a control call, cancels, and observes the registration still reaching the wrapped backend. Neither did the lane that filed it. That test is the first thing the fix needs, and per AGENTS.md it must fail before the fix and pass after — a test asserting only "cancelPending rejected the completion" passes today and proves nothing about this.

Note for whoever takes it: the fix touches the same _control queue that #649 (open) reasons about. #649's lane checked and reported that its change "neither helps nor hinders it — the queued _control task is still inner->bindModelBlocking(...) and cancelPending is untouched." I have not independently confirmed that, but it means this can be taken independently of #649 landing, not that it should be taken concurrently with it — both touch backend.hpp.

What would change the verdict

Close as invalid if a test shows the queued task already declines to run after cancelPending — i.e. if some existing guard on _control covers it and the header comment is pessimistic.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW