-
Notifications
You must be signed in to change notification settings - Fork 909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recover from duplicate class definition errors #5185
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
this issue has caused headaches for injecting SPI classes (since ideally we would inject them when we see the SPI interface loaded) e.g.
Lines 55 to 58 in 88121c1
// we cannot use ContextDataProvider here because one of the classes that we inject implements | |
// this interface, causing the interface to be loaded while it's being transformed, which | |
// leads to duplicate class definition error after the interface is transformed and the | |
// triggering class loader tries to load it. |
I'm in favor of adding the ThreadLocal check if that doesn't add too much complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx!
mv.visitMethodInsn( | ||
Opcodes.INVOKESTATIC, | ||
Type.getInternalName(DefineClassContext.class), | ||
"enter", | ||
"()V", | ||
false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you update the pseudo code above (which is really helpful) with the new enter/exit/exitAndGet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx again, I'm looking forward to trying this out by removing a couple of SPI interface instrumentation hacks
} catch (Throwable throwable) { | ||
DefineClassContext.exit(); | ||
throw throwable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* Recover from duplicate class definition errors * fix hotspot8 * Suppress dupicate class definiton errors only when helper classes were injected * exit define class context when there is an exception, update pseudocode in comment Co-authored-by: Lauri Tulmin <[email protected]>
* Recover from duplicate class definition errors * fix hotspot8 * Suppress dupicate class definiton errors only when helper classes were injected * exit define class context when there is an exception, update pseudocode in comment
Resolves #5155
If there is a duplicate class definition find the class with
findLoadedClass
and return it instead of throwing aLinkageError
. Currently no attempt is made to detect where the duplicate definition was caused by us or not. If needed we could add aThreadLocal
to detect whether helper classes were injected while defining a class and only suppress duplicate class definition errors then.