rpc: discover DynamicDispatchRpcCodec providers across classloaders#7698
Merged
Conversation
The static initializer ran ServiceLoader.load once against whichever classloader happened to load DynamicDispatchRpcCodec first. In the mod CLI, that classloader holds rewrite-core but cannot see the recipe plugin classloader holding rewrite-csharp (and other language jars), so CODEC_BY_TYPE stayed empty for the JVM lifetime. With no codec found, RpcSendQueue fell back to inline Jackson JSON for Cs.CompilationUnit; the C# side then failed deserializing into sealed primary-ctor records, surfacing as `Unable to cast Newtonsoft.Json.Linq.JObject to Tree` at RewriteRpcServer.GetObjectFromRemoteAsync. Replace the one-shot load with a lazy discoverFrom(ClassLoader) that fires for the defining classloader at static init and for the current thread's context classloader plus the candidate object's classloader on every getCodec call. A WeakHashMap-backed set ensures each classloader is scanned at most once and providers are deduped by concrete class so repeat scans don't accumulate duplicates.
Contributor
|
An alternative could be to make the language |
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
3 participants
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
DynamicDispatchRpcCodec's static initializer ranServiceLoader.load(DynamicDispatchRpcCodec.class)once, against whichever classloader happened to load the class. Whenrewrite-coreis loaded by a bootstrap/parent classloader and language jars (rewrite-csharp,rewrite-python, …) are loaded by recipe/plugin classloaders that aren't visible to the bootstrap CL,CODEC_BY_TYPEends up empty for the JVM lifetime. With no codec found,RpcSendQueuefalls back to inline Jackson JSON for the whole tree; the receiving side then fails to deserialize complex tree types and surfaces it as aJObject → Treecast error.This PR replaces the one-shot load with a lazy
discoverFrom(ClassLoader)that:DynamicDispatchRpcCodec.class.getClassLoader()at static init, so co-located codecs still work in single-classloader topologies.getCodec(...)call, also scansThread.currentThread().getContextClassLoader()and the candidate object's classloader if either hasn't been scanned yet.WeakHashMap-backed set so each is scanned at most once.Test plan
rewrite-coreand a language jar load via different classloaders — observedCODEC_BY_TYPE.keySet() == []with the pre-fix code and the expectedCs.CompilationUnitprint failure (codec missed → inline JSON → Newtonsoft can't construct the sealed primary-ctor record).CODEC_BY_TYPElazily on firstgetCodecinvocation and the round-trip succeeds.rewrite-csharp:testsuite still passes — single-classloader test harness behaviour is unchanged because the static-init scan still runs against the defining classloader.