Detect stream EOF in reader loop instead of spinning#16
Merged
Conversation
When an RPC peer dies, its stdout pipe goes to permanent EOF. The previous reader loop treated EOF as an empty header line, synthesized a JsonRpcError with no id, constructed a JsonRpcException, looped, and repeated -- pegging a CPU core for the lifetime of the JVM. Observed in production where rewrite-marketplace burned ~76% of one core for 20+ hours filling stack traces after npx exited at startup. The handlers now distinguish "stream closed" (EOFException) from "malformed message on a live stream" (recoverable JsonRpcError). The JsonRpc reader loop catches EOFException once, fails any in-flight requests, and exits. Also guards the no-id error broadcast: skip the JsonRpcException allocation entirely when there are no open requests to fail.
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
When an RPC peer dies, its stdout pipe goes to permanent EOF. The previous reader loop treated
read() == -1as an empty header line, synthesized aJsonRpcErrorwith no id, constructed aJsonRpcException(paying fullfillInStackTrace), looped, and repeated -- pegging one core for the lifetime of the JVM.This was observed in production:
recipe-marketplacerannpx --package=@openrewrite/rewrite@<version> rewrite-rpcat boot. The pinned npm version had not been published, so npx exited 1 immediately. The JVM-side reader thread spun for the full 20.5h of marketplace uptime, with 76% of total CPU inThrowable.fillInStackTrace.Changes:
MessageHandler.receivenow declaresthrows IOExceptionso handlers can surface stream-closed cleanly instead of swallowing it as a recoverable parse error.HeaderDelimitedMessageHandlerandNewLineDelimitedMessageHandler: throwEOFExceptionon first-byte EOF (between messages) and on mid-message EOF, instead of returning""/ a malformed-content error.JsonRpc.bind(): catchEOFExceptiononce, fail any open requests, setshutdown = true, exit the loop.JsonRpc.bind()now skips theJsonRpcExceptionallocation entirely whenopenRequestsis empty -- so a single noisy non-RPC line on the wire (still possible for live but chatty peers) costs nothing when nothing is in flight.API impact:
MessageHandleris a small, library-internal interface; the only implementors live in this repo. Verified thatorg.openrewrite:rewrite-core(the only upstream non-library consumer of this API) does not implementMessageHandlerand does not call.receive(...)directly, so this is a transparent bump for downstream.Test plan
./gradlew testpassesHeaderDelimitedMessageHandlerTestcovers: EOF before any byte, malformed-but-non-empty header still returns recoverableJsonRpcError, mid-message EOF surfaces asEOFExceptionJsonRpcTest.readerLoopExitsCleanlyOnEofcovers the full reader-loop teardown path: an in-flight request fails (rather than hangs) when the peer closes the stream