Repository navigation
rmi: split Decoder opening for server-side flows - #21
Merged
Merged
Conversation
Opening() reads handshake+ClientEndpoint in one call, which deadlocks on
a server-side live reader: a conforming Java client blocks after its
7-byte handshake waiting for the server's ProtocolAck before it sends
the endpoint echo. Servers need to inject a write between the two reads,
which the unified Opening() call forbade.
Split the opening phase into three mutually exclusive primitives:
ReadHandshake (server read), ReadClientEndpoint (server read after Ack
write), and ReadAcknowledge (client read). A stage enum enforces call
ordering. Opening() remains the one-shot entry point for fully-buffered
captures and client-direction reads; Next() still auto-advances from
any stage to stageReady so bare captures just work.
Also add ToBytes encoders on Handshake/Acknowledge/Endpoint (zero-value
Magic/Protocol/Flag default to JRMI_MAGIC/ProtocolStream/AckFlag so
server code can write &Acknowledge{Host, Port}.ToBytes() without
ceremony), and a ToString on Endpoint for symmetry with the other two.
Tests: TestDecoderServerFlowWithInterleavedAck uses two io.Pipes to
reproduce the real client timing and assert no deadlock. Boundary tests
cover the stage-machine rejections, Next() auto-advance, and encoder
round-trip. ExampleDecoder_serverFlow demonstrates the new sequence.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Shrink ~75 lines to ~30. Cut what the code already says (per-frame field breakdowns, the maybeReadClientEndpoint hostname-length edge case, the shared-cursor note, a redundant restatement of Decoder.Next behavior, the full client-side Opening() example that's already in the Decoder godoc). Keep what future work needs to avoid landmines: Registry-only scope + dispatch gate + JDK recalibration hint, FromBytes vs Decoder deadlock, both server-side Opening() and readReturn sentinel traps on live readers, the no-end-marker invariant that forbids replacing the sentinel with serz.FromBytes, and the wireshark-dump anti-pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Decoder's opening phase into three mutually exclusive primitives (ReadHandshake/ReadClientEndpoint/ReadAcknowledge) so an RMI Registry server can write itsProtocolAckbetween the two client reads. The existingOpening()stays as the one-shot entry point for fully-buffered captures and client-direction reads.ToBytes()encoders onHandshake/Acknowledge/Endpointso servers can write framing (&Acknowledge{Host, Port}.ToBytes()) without reimplementing the modified-UTF layout. Also addEndpoint.ToString()for symmetry.ExampleDecoder_serverFlowdemonstrating the new read ordering and updateCLAUDE.md's Live-TCP section.Why
Opening()readsHandshake + ClientEndpointin one call. A conforming Java client (sun.rmi.transport.tcp.TCPChannel) sends its 7-byte handshake and then blocks waiting for the server'sProtocolAckbefore writing the endpoint echo. On a server-side livenet.Conn, the peek insideOpening()waits for bytes the client will never send, so the caller deadlocks. There was no way to implement a working Registry server on top ofDecoderwithout this split.Design
decoderStageenum (stageInitial→stageAfterHandshake→stageReady) guards call ordering: the threeRead*primitives andOpening()are mutually exclusive, and each returns an error if called out of order.Next()auto-advances from either earlier stage tostageReady, so bare captures (no handshake) and callers who skipReadClientEndpointafterReadHandshakestill work transparently.ToBytes()encoders default zero-valuedMagic/Protocol/FlagtoJRMI_MAGIC/ProtocolStream/AckFlagso construction from scratch stays ergonomic; round-trip fidelity viaToBytes(FromBytes(x))is unaffected because parsing rejects bad values before they reach the struct.Test plan
TestDecoderServerFlowWithInterleavedAck— reproduces the real client timing using twoio.Pipepairs (client writes handshake, blocks onio.ReadFullof the Ack, then writes endpoint + Ping); asserts no deadlock and correct field values at each stage.TestDecoderOpeningAfterReadHandshakeErrors,TestDecoderReadHandshakeAfterOpeningErrors,TestDecoderReadClientEndpointRequiresHandshake,TestDecoderNextAutoConsumesClientEndpoint,TestDecoderReadAcknowledge.TestOpeningEncodersRoundTrip— byte-exact match of the threeToBytes()outputs against the existing fixture builders.TestEndpointToString— wireshark-dissector format with decimal + hex for length/value/port.ExampleDecoder_serverFlowrunnable example; all pre-existing examples and integration tests green (go test ./...).🤖 Generated with Claude Code