Files
logos-protocol/cpp/implementations/plain
Dario LipicarandClaude Opus 5 4db061aba3 fix(plain): close the RPC socket on the connection's strand, not the caller's thread (#38)
* fix(plain): close the RPC socket on the connection's strand

RpcConnection<Stream>::fail() closed the socket on whatever thread called
it. Every other access to m_stream is serialized on m_strand — start()
and writeFrame() post onto it, doRead()/doWrite() complete through
bind_executor(m_strand, ...) — but a strand serializes handlers, not a
raw call made from outside it, and asio sockets are documented as unsafe
for concurrent use.

Consumer teardown (~RpcClient -> ~PlainTransportConnection -> stop() ->
fail()) therefore ran close() -> cleanup_descriptor_data(), nulling
impl.reactor_data_, while the io worker thread was inside
reactive_socket_service_base::start_op() for a doWrite() that had just
been posted. start_op()'s 'descriptor_data' is a reference to that member:
the null check passes before the store lands, then the shutdown_ read
after it dereferences null. SIGSEGV at +0x98 on the IoContextPool thread.

fail() now hands the close to the strand via boost::asio::dispatch, which
runs it inline when fail() is already on the strand (the io-thread error
path, unchanged behaviour) and queues it otherwise. dispatch never
blocks, so teardown cannot deadlock or hang; the lambda holds a
shared_ptr so a close queued from a destructor still finds a live object.

writeFrame()'s m_stopped check is also repeated inside the posted lambda
and in doWrite(): the outer load is only a hint, and fail() can land
between it and the handler.

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

* test(plain): a teardown-race regression that also guards against leaks and hangs

Hammers the shape that crashed: a consumer connection with frames still
queued is destroyed from its own thread, 400 times over, while the io
worker is initiating the async_write for a just-posted frame. Pre-fix
this takes the whole test binary down inside asio's reactor; post-fix the
close runs on the strand and can never overlap a write initiation.

The same loop is the guard for the two things the fix could plausibly
break: the descriptor count must come back (an async close that never
runs would strand fds) and the loop must finish promptly (a close that
blocked on the io thread would show up as a stall).

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

* fix(plain): stop dispatching inbound frames once the connection failed

Moving the close onto the strand left the socket open between stop()
returning and the strand getting to it. A frame that arrived in that gap
still ran through handleFrame -> dispatchIncoming and into the
IncomingCallHandler — which, on the host side, the caller may already be
in the middle of destroying (RpcServer::stop() runs from
~PlainTransportHost). Before the close moved, the immediate close aborted
the read and that frame never landed.

The connection is torn down either way: every pending promise has already
been failed and every event callback cleared, so there is nothing a late
frame could usefully resolve. Drop it.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-31 14:46:51 -03:00
..