Fix RPC framework errors caused by concurrent visitor cursor access#6880
Merged
knutwannheden merged 5 commits intomainfrom Mar 5, 2026
Merged
Fix RPC framework errors caused by concurrent visitor cursor access#6880knutwannheden merged 5 commits intomainfrom
knutwannheden merged 5 commits intomainfrom
Conversation
Two fixes for errors seen in production recipe runs against JS/TS repos: 1. TreeVisitor.visit(): Use local variables for cursor management instead of reading from the mutable `cursor` field, which can be clobbered when singleton sender visitors are shared across ForkJoinPool threads. The field writes via setCursor() are preserved so getCursor() in visitor methods still works. 2. GetObject.Handler: Remove stale remoteObjects entry on send failure, so the next getObject() call sends a full ADD instead of a CHANGE delta against a partially-sent baseline.
DynamicDispatchRpcCodec subclasses are singletons (loaded via ServiceLoader), but previously some held singleton sender/receiver visitor fields with mutable cursor state. Under concurrent access from GetObject.Handler's ForkJoinPool, this caused NPEs. Create new sender/receiver instances per rpcSend/rpcReceive call, matching the pattern already used by Python, JavaScript, and C# codecs.
…types The parameterizedSignature() method added to the cycle-detection stack but never checked the return value, so revisiting the same Parameterized through MultiCatch or Intersection types caused infinite recursion. - Check parameterizedStack.add() return value and short-circuit on cycle - Use try/finally to ensure stack cleanup even on exceptions - Add cycle protection to multiCatchSignature() and intersectionSignature() - Add tests for cyclic Parameterized→MultiCatch and self-referencing types
… cyclic types" This reverts commit d15873d.
These will be revisited separately.
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
remoteObjectsinGetObject.Handlercatch block to prevent stale baseline on retry after send failureDynamicDispatchRpcCodecsubclasses, eliminating thread-safety issues from shared mutable visitor stateTest plan
gw :rewrite-core:test --tests "org.openrewrite.rpc.RewriteRpcTest"— tests for remoteObjects cleanup on send failure and baseline reuse