mirror of
https://github.com/logos-co/logos-protocol.git
synced 2026-09-02 06:51:14 +00:00
Dario Gabriel LipicarandClaude Opus 5
1e7c27af10
rpc_connection.h claimed that "a caller that gives up before the reply arrives
calls cancelPending() and is never called back at all". It is not true.
dispatchIncoming copies the handler out of m_pendingCalls under m_mu and invokes
it with the mutex RELEASED, so a cancelPending() landing in that gap erases an
entry that is already gone and returns having stopped nothing — the handler then
runs to completion, after cancelPending() has returned.
THE CODE IS FINE; THE COMMENT WAS NOT, and the distinction it was blurring is
the load-bearing one:
* at-most-once INVOCATION of a registered handler is this layer's, and comes
from the extract-and-erase under m_mu — three contenders (dispatchIncoming,
fail()'s sweep, cancelPending) and only one can have it;
* exactly-once DELIVERY to the user is NOT this layer's. It is
AsyncCall::deliver()'s CAS, and nothing else.
Both callers of sendCallAsync() were checked, because a non-idempotent one would
have made this a bug rather than a comment. PlainLogosObject funnels every
outcome into deliver(). RpcConnection::sendCall()'s promise handler has no CAS
and needs none: only one contender ever reaches it, and fulfilling a future its
caller has already walked away from is a no-op. Same for the two cancelPending()
callers that are not deliver() — the sync callMethodWithError timeout and
getMethods().
Three comment sites corrected (ResultHandler, dispatchIncoming's Result arm,
cancelPending's own contract) and the claims moved out of prose into
tests/protocol/test_plain_cancel_pending_race.cpp, which BUILDS the interleaving
instead of racing for it: a stub connection reproduces dispatchIncoming's
extract-then-invoke and lets the test stand between the halves. Four tests — the
callback that fires after cancelPending() returns, the extracted reply racing
teardown (exactly one delivery), the promise-shaped handler in the same gap, and
a real RpcConnection pair proving the half of the old comment that IS true.
The exactly-once one goes RED under -DLOGOS_PROTOCOL_DETECTOR_INVERSIONS=ON:
2 deliveries for 1 call, deterministically rather than probabilistically. Six
tests now go red in that build, listed in tests/protocol/CMakeLists.txt. Full
suite 302/302.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>