[
https://issues.apache.org/jira/browse/GROOVY-11978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18077752#comment-18077752
]
ASF GitHub Bot commented on GROOVY-11978:
-----------------------------------------
Copilot commented on code in PR #2504:
URL: https://github.com/apache/groovy/pull/2504#discussion_r3175977209
##########
src/main/java/org/codehaus/groovy/tools/RootLoader.java:
##########
@@ -109,8 +113,16 @@ public RootLoader(final LoaderConfiguration lc) {
Thread.currentThread().setContextClassLoader(this);
+ // Skip URLs already on the bootstrap (system) classpath: the launcher
+ // puts the core groovy jar on -classpath to start GroovyStarter, and
+ // groovy-starter.conf's "load lib/*.jar" then re-globs it. Adding the
+ // same jar to both loaders lets each define its own copy of every core
+ // class, and trips duplicate-resource detection.
+ Set<File> bootstrap = bootstrapClasspathFiles();
Review Comment:
The new logic is based on `System.getProperty("java.class.path")`, which is
the application/system classpath, not the JVM bootstrap classpath. To avoid
confusion for future maintainers, consider renaming
`bootstrapClasspathFiles`/`bootstrap` and adjusting the comment to refer to the
system/application classpath explicitly.
##########
src/main/java/org/codehaus/groovy/tools/RootLoader.java:
##########
@@ -179,4 +191,33 @@ public void addURL(final URL url) {
protected Class<?> findClass(final String name) throws
ClassNotFoundException {
throw new ClassNotFoundException(name);
}
+
+ private static Set<File> bootstrapClasspathFiles() {
+ Set<File> files = new HashSet<>();
+ String cp = System.getProperty("java.class.path");
+ if (cp == null || cp.isEmpty()) return files;
+ for (String entry : cp.split(File.pathSeparator)) {
+ if (entry.isEmpty()) continue;
+ File f = canonicalFile(new File(entry));
+ if (f != null) files.add(f);
+ }
+ return files;
+ }
+
+ private static File canonicalFile(URL url) {
+ if (!"file".equals(url.getProtocol())) return null;
+ try {
+ return canonicalFile(new File(url.toURI()));
+ } catch (URISyntaxException | IllegalArgumentException e) {
+ return null;
+ }
+ }
+
+ private static File canonicalFile(File f) {
+ try {
+ return f.getCanonicalFile();
+ } catch (IOException e) {
Review Comment:
`File#getCanonicalFile()` may also throw `SecurityException` if read access
is denied. Consider catching `SecurityException` alongside `IOException` and
falling back to `getAbsoluteFile()` to keep behavior consistent with the intent
of this helper.
##########
src/main/java/org/codehaus/groovy/tools/RootLoader.java:
##########
@@ -179,4 +191,33 @@ public void addURL(final URL url) {
protected Class<?> findClass(final String name) throws
ClassNotFoundException {
throw new ClassNotFoundException(name);
}
+
+ private static Set<File> bootstrapClasspathFiles() {
+ Set<File> files = new HashSet<>();
+ String cp = System.getProperty("java.class.path");
Review Comment:
`System.getProperty("java.class.path")` can throw `SecurityException` in
restricted environments. Consider catching `SecurityException` here (and
treating it like an empty classpath) so RootLoader construction doesn't fail
under a security manager / sandboxed execution.
> RootLoader duplicates the core jar already on the bootstrap classpath
> ---------------------------------------------------------------------
>
> Key: GROOVY-11978
> URL: https://issues.apache.org/jira/browse/GROOVY-11978
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> The launcher script puts {{groovy-x.y.z.jar}} on the JVM bootstrap
> {{-classpath}} so that {{GroovyStarter}} can run. {{GroovyStarter}} then
> reads {{groovy-starter.conf}}, whose {{load !\{groovy.home}/lib/*.jar}} glob
> picks up the same core jar again and adds it to the {{RootLoader}}.
> The result is that every core class is reachable via two defining loaders
> (the system classloader and the {{RootLoader}}), and
> {{ClassLoader.getResources}} returns the core jar twice. This trips the
> duplicate-module check in {{MetaClassRegistryImpl}}, which logs a warning on
> every standard launch listing the same jar URL twice.
> The fix is to skip URLs already on {{java.class.path}} when populating the
> {{RootLoader}} from a {{LoaderConfiguration}}, so core classes are owned by
> the system classloader and module/grab jars are owned by the {{RootLoader}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
