Repository navigation
Conversation
The previous FromStream refused non-Registry Calls and all ReturnData because it needed exact arg counts to avoid blocking at frame boundaries. That worked, but left a usability gap: any live-TCP consumer of non- Registry remotes had to buffer the whole conversation and fall back to FromBytes, which defeats the point of a streaming parser. This change unifies the arg/payload strategy and adds a proper frame-by- frame API: - readCallArgs now picks strategy from the frame's own header, not from the input source. Registry fast path (ObjID + interface hash + known op) still uses exact-count — no peek past the last arg, so Registry Calls over a live net.Conn return as soon as their own bytes arrive. Every other Call falls back to the sentinel (peek next byte, stop on non-TC_* / EOF). - readReturn drops the streaming-refusal and always uses the sentinel for its 0/1-payload count. ReturnData now parses in streaming mode. - The `streaming bool` parameter is removed from parseTransmission, readMessage, readCall, readReturn, readCallArgs. FromStream is a thin wrapper over the same parseTransmission that FromBytes uses. - New rmi.Decoder (rmi/decoder.go) reads one frame per Next() call, with an optional Opening() to consume the handshake prefix. This is the idiomatic API for live net.Conn consumers: the caller applies SetReadDeadline between Next() calls to bound how long to wait for the next frame. Non-Registry Calls block on the sentinel's terminating peek — documented as a protocol-inherent tradeoff, not a bug. Tests: - Four TestStreamRejects* tests replaced with positive counterparts that exercise the new sentinel path on bytes.Reader input. - decoder_test.go adds 7 tests including two blocking-contract tests: TestDecoderRegistryCallReturnsWithoutPeek proves Registry Calls return without peeking past the last arg (uses a never-closed io.Pipe with a 2s deadlock guard); TestDecoderNonRegistryCallBlocksOnSentinelPeek asserts the expected blocking behavior for non-Registry Calls and that the parser unblocks correctly on io.EOF. CLAUDE.md's rmi section rewritten: "Streaming scope" (the list of what FromStream rejected) is replaced by "Arg-reading strategy" (how each frame picks its own reader) and "Live-TCP ergonomics" (Decoder usage with SetReadDeadline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
Superseded by #NEW — this PR's direction ('support all frame types in streaming mode') turned out to be the wrong call. The new PR takes the opposite tack: restrict the module to java.rmi.registry.Registry traffic for clarity, and ship the frame-by-frame Decoder API without the non-Registry sentinel baggage. |
3 tasks done
Owner
Author
|
Superseded by #20. |
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
FromStream's Registry-only restriction: non-RegistryCallMessageandReturnMessagenow parse in streaming mode via sentinel fallback. Registry Calls keep their exact-count fast path, so the common case still returns without peeking past the last arg.rmi.Decoder— an idiomatic frame-by-frame API (NewDecoder+Opening+Next) for livenet.Connconsumers who want to applySetReadDeadlinebetween frames.streaming boolparameter chain (parseTransmission→readMessage→readCall/readReturn/readCallArgs). Each frame picks its arg strategy from its own header, not from the input source.Why
Before this change, any live-TCP consumer of a non-Registry remote — or any consumer of
ReturnMessage— had to buffer the whole conversation and callFromBytes, which defeats the point of a streaming parser. The unified sentinel approach + per-frame Decoder API solves that without losing the Registry non-blocking fast path.The one genuinely new tradeoff is documented up front: non-Registry Calls and Returns block on a terminating peek until the next frame arrives or the reader returns an error. That's protocol-inherent (JRMP has no length prefix inside Call/Return), and the Decoder's contract is to push deadline management up to the caller via
conn.SetReadDeadline, same pattern mature HTTP/WebSocket servers use.Test plan
go test ./...— all 80+ existing tests pass, plus new coverage:TestStream{NonRegistryCall,WrongInterfaceHash,UnknownRegistryOp,ReturnWithPayload,ReturnVoid}— replacements for the four oldTestStreamRejects*testsTestDecoderFrameByFrame,TestDecoderSkipsOpeningWhenOmitted,TestDecoderOpeningTwiceErrors,TestDecoderOpeningAfterNextErrors,TestDecoderBareCaptureNoOpeningTestDecoderRegistryCallReturnsWithoutPeek— feeds exact Registry Call bytes into anio.Pipethat is never closed; if the parser peeks past the last arg it deadlocks and a 2s timeout fires the failure. Proves the exact-count fast path.TestDecoderNonRegistryCallBlocksOnSentinelPeek— asserts the sentinel peek does block while the pipe is open (150ms budget) and unblocks correctly once the writer closes. Documents the blocking contract.go vet ./...golangci-lint run(local version too new for current config; CI will validate)go build ./...🤖 Generated with Claude Code