fix: fix class and resource loading in maven plugin #20344
Conversation
Introduces a custom class loader for ClassFinder to ensure resources and classes are loaded first from project dependencies and then from the plugin classloader. Also, augments plugin classloader adding URLs of project artifacts to prevent class cast and class not found exceptions. Fixes #19616 Fixes #19009
|
Tested with maven 3.6.3, 3.8.6, 3.9.9 and 4.0.0-beta-4; seems to work correctly. |
|
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private void resetPluginClassLoader(ClassRealm classRealm) { |
There was a problem hiding this comment.
Could we do the reset instead by customizing URLClassLoader to support removeUrl.
ClassRealm seems to call super.addURL, which is implemented in URLClassLoader. URLCLassLoader uses internally URLClassPath field called ucp, which could be overridden by reflection to temporarily add support to remove urls that were added temporarily.
ClassRealm has already addUrl and getUrls() which means we can check beforehand which urls were actually added.
If I have understood this right, doing reset like this would mean that we don't have to mind caching.
There was a problem hiding this comment.
Interesting idea.
I suppose we can't just replace the ClassRealm ucp because some URLs might already have been loaded, making it more difficult to detect which URLs should be removed after execution.
But we can probably have a URLClassPath decorator with the added URLs, that first loads from itself and then delegates to the original ucp.
I'll give it a try.
There was a problem hiding this comment.
It looks like URLClassPath is from jdk.internal.loader, so not exported to external modules.
There was a problem hiding this comment.
OK, so reflection doesn't work. I saw this VM argument to enable it for ClassLoader where ucp apparently used to be in older Java versions: --add-opens java.base/jdk.internal.loader=ALL-UNNAMED.
But even if it could help, you are probably right it's not helping really as URLs might be already loaded and cached anyway.
This method is still something I would rather not add except as last option, but I don't have more suggestions atm.
There was a problem hiding this comment.
--add-opens should be added by the end user in MAVEN_OPTS or similar, so it would also be some sort of breaking change.
Honestly, I run out of ideas on how to fix class loading issues in less tricky ways.
Forking the JVM to execute mojo logic might help, but my guess is that it will bring in many other kinds of problems.
There was a problem hiding this comment.
PluginRealmCache is internal non-public API, which could change on every maintenance release of the maven-core in worst case scenario.
Maybe it would make sense to give a try with "forking JVM" way, especially if it could work for example easier with prepare-frontend goal first to start with. Assuming there would be differences between goals and prepare-frontend is simplest.
But anyway if we keep this method, I would like it to be extracted in internal utility class as a static method, and also split to methods for handling mvnd and default reset logic separately.
Description
Introduces a custom class loader for ClassFinder to ensure resources and classes are loaded first from project dependencies and then from the plugin classloader. Also, augments plugin classloader adding URLs of project artifacts to prevent class cast and class not found exceptions.
Fixes #19616
Fixes #19009
Type of change
Checklist
Additional for
Featuretype of change