fix: deduplicate methods in JavassistProxy to prevent DuplicateMemberException#1554
fix: deduplicate methods in JavassistProxy to prevent DuplicateMemberException#1554sunhailin-Leo wants to merge 1 commit intomasterfrom
Conversation
…Exception (#1384) When an interface extends multiple parent interfaces that declare the same method signature, interfaceClass.getMethods() returns duplicate Method objects. JavassistProxy.createMethod() previously added all of them without deduplication, causing Javassist to throw DuplicateMemberException when attempting to add the same method twice to the generated proxy class. Fix: introduce addedMethodSignatures (HashSet<String>) to track method signatures in the format 'methodName(paramType1,paramType2,...)'. Before processing each method, check if its signature has already been seen; if so, skip it. This ensures each unique method signature is only added once to the proxy class, regardless of how many parent interfaces declare it. Fixes #1384
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change adds method-signature deduplication to Javassist proxy generation. When interfaces inherit identical methods from multiple parent interfaces, the proxy generator now tracks method signatures in a Set and skips duplicates, preventing Javassist from throwing DuplicateMemberException. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes Javassist-based proxy generation when service interfaces inherit duplicate method signatures from multiple parent interfaces (issue #1384), preventing DuplicateMemberException during proxy class creation.
Changes:
- Add a
Setto deduplicate methods while iteratinginterfaceClass.getMethods()increateMethod(). - Introduce a helper
buildMethodSignature(Method)used as the deduplication key. - Add supporting imports (
HashSet,Set) and inline documentation referencing #1384.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private String buildMethodSignature(Method method) { | ||
| StringBuilder signature = new StringBuilder(method.getName()); | ||
| signature.append('('); | ||
| Class<?>[] parameterTypes = method.getParameterTypes(); | ||
| for (int i = 0; i < parameterTypes.length; i++) { | ||
| if (i > 0) { | ||
| signature.append(','); | ||
| } | ||
| signature.append(parameterTypes[i].getName()); | ||
| } |
There was a problem hiding this comment.
The dedup key in buildMethodSignature() ignores the return type, but JVM/Javassist method uniqueness is based on name + full descriptor (including return type). With covariant return types (or compiler bridge methods), two methods can legally share the same name/parameter types but have different return types; this dedup would incorrectly drop one and may leave the generated proxy missing a required interface method or with the wrong return type. Include the return type in the signature (or derive the JVM descriptor) so only true duplicates are skipped.
| // Track method signatures to skip duplicate methods from multiple parent interfaces. | ||
| // When an interface extends multiple parent interfaces that declare the same method, | ||
| // interfaceClass.getMethods() returns duplicates, causing Javassist to throw | ||
| // DuplicateMemberException when adding the same method twice. | ||
| // See: https://github.com/sofastack/sofa-rpc/issues/1384 | ||
| Set<String> addedMethodSignatures = new HashSet<String>(); | ||
| for (Method m : methodAry) { | ||
| mi++; | ||
| if (Modifier.isNative(m.getModifiers()) || Modifier.isFinal(m.getModifiers()) || | ||
| Modifier.isStatic(m.getModifiers())) { | ||
| continue; | ||
| } | ||
| // Skip duplicate methods inherited from multiple parent interfaces | ||
| if (!addedMethodSignatures.add(buildMethodSignature(m))) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This change adds new behavior (skipping duplicate inherited methods) but there’s no test exercising the failure mode from #1384 (diamond interface inheritance with duplicate method signatures). Since this module already has JavassistProxyTest, consider adding a test interface that extends two parents declaring the same method and assert getProxy() succeeds (and the method invocation is routable) to prevent regressions.
What problem does this PR solve?
Fixes #1384
Root Cause
When a service interface extends multiple parent interfaces that declare the same method signature (e.g. both
SubInterface1andSubInterface2declareString getType()),interfaceClass.getMethods()returns duplicateMethodobjects — one for each parent interface.JavassistProxy.createMethod()previously iterated over all returned methods without any deduplication, causing Javassist to callmCtc.addMethod()with the same method signature twice, which throws:Switching to ByteBuddy proxy worked as a workaround because ByteBuddy handles deduplication internally.
Solution
Introduce
addedMethodSignatures(HashSet<String>) increateMethod()to track method signatures in the formatmethodName(paramType1,paramType2,...). Before processing each method, check if its signature has already been seen; if so, skip it. This ensures each unique method signature is only added once to the generated proxy class.Reproduce
Before fix: throws
SofaRpcRuntimeExceptionwrappingDuplicateMemberExceptionAfter fix: proxy generated successfully
Changes
core-impl/proxy/src/main/java/com/alipay/sofa/rpc/proxy/javassist/JavassistProxy.javaimport java.util.HashSetandimport java.util.SetaddedMethodSignaturesset increateMethod()for deduplicationbuildMethodSignature(Method)Summary by CodeRabbit