-
Notifications
You must be signed in to change notification settings - Fork 571
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
HV-1950 Suppress java.lang.NoClassDefFoundError: com/sun/el/ExpressionFactoryImpl #1318
base: main
Are you sure you want to change the base?
Conversation
LOG.debug( "Loaded expression factory via com.sun.el classloader" ); | ||
return expressionFactory; | ||
try { | ||
run( SetContextClassLoader.action( Class.forName( "com.sun.el.ExpressionFactoryImpl" ).getClassLoader() ) ); |
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.
Hey, thanks for submitting this PR.
@CallerSensitive
public static Class<?> forName(String className)
throws ClassNotFoundException {
Class<?> caller = Reflection.getCallerClass();
return forName0(className, true, ClassLoader.getClassLoader(caller), caller);
}
Since that's how the Class.forName
is implemented wouldn't we just end up having the same class loader as in the very first try of this buildExpressionFactory()
? This part:
if ( canLoadExpressionFactory() ) {
ExpressionFactory expressionFactory = ELManager.getExpressionFactory();
LOG.debug( "Loaded expression factory via original TCCL" );
return expressionFactory;
}
It would also be nice if the changes were accompanied by a test case; thanks!
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.
I would refrain from changing the logic in there. It is very brittle and we worked around multiple issues with OSGi and the modular classpath from WildFly.
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.
I've revert to ExpressionFactoryImpl.class.getClassLoader()
, just catch NoClassDefFoundError
and ignore it.
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.
Since that's how the
Class.forName
is implemented wouldn't we just end up having the same class loader as in the very first try of thisbuildExpressionFactory()
? This part:
ExpressionFactoryImpl
is load from ResourceBundleMessageInterpolator
's classloader, but its actual classloader could be another classloader due to delegation.
…nFactoryImpl It will mislead user that hibernate validator will rely on particular EL implementation.
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
It will mislead user that hibernate validator will rely on particular EL implementation.
see https://hibernate.atlassian.net/browse/HV-1950.