Skip to content

Commit 347c951

Browse files
mcollovatimcollovati
authored
fix: Prevent stale JAR cache in Reflector under Maven daemon (mvnd) (#23863) (#23987)
Mirror the Gradle daemon fix (33fa374) in the Maven plugin's `Reflector` and `ReflectorClassLoader`/`CombinedClassLoader`: - Extract `ReflectionsClassFinder.disableJarCaching()` as a public utility so both plugins can reuse it. - Wrap `jar:` URLs in `ReflectorClassLoader.getResource()` and `getResources()` (flow-maven-plugin) and `CombinedClassLoader` (flow-dev-bundle-plugin) with `useCaches(false)` to prevent stale `JarFileFactory` entries across daemon builds. - Make `Reflector` implement `Closeable` with a `close()` method that releases the `URLClassLoader` file handles, and register a `Cleaner` action for best-effort GC cleanup of abandoned instances. - Close the temporary `Reflector` in `FlowModeAbstractMojo.isHillaAvailable(MavenProject)` via try-with-resources. Releated to #15458
1 parent 3a01880 commit 347c951

File tree

5 files changed

+138
-17
lines changed

5 files changed

+138
-17
lines changed

‎flow-plugins/flow-dev-bundle-plugin/src/main/java/com/vaadin/flow/plugin/maven/Reflector.java‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
*/
1616
package com.vaadin.flow.plugin.maven;
1717

18+
import java.io.Closeable;
1819
import java.io.File;
1920
import java.io.IOException;
21+
import java.lang.ref.Cleaner;
2022
import java.lang.reflect.Field;
2123
import java.lang.reflect.Modifier;
2224
import java.net.URL;
@@ -48,7 +50,7 @@
4850
/**
4951
* Helper class to deal with classloading of Flow plugin mojos.
5052
*/
51-
public final class Reflector {
53+
public final class Reflector implements Closeable {
5254

5355
public static final String INCLUDE_FROM_COMPILE_DEPS_REGEX = ".*(/|\\\\)(portlet-api|javax\\.servlet-api)-.+jar$";
5456
private static final Set<String> DEPENDENCIES_GROUP_EXCLUSIONS = Set.of(
@@ -60,6 +62,7 @@ public final class Reflector {
6062
"org.zeroturnaround:zt-exec:jar");
6163
private static final ScopeArtifactFilter PRODUCTION_SCOPE_FILTER = new ScopeArtifactFilter(
6264
Artifact.SCOPE_COMPILE_PLUS_RUNTIME);
65+
private static final Cleaner CLEANER = Cleaner.create();
6366

6467
private final URLClassLoader isolatedClassLoader;
6568
private List<String> dependenciesIncompatibility;
@@ -73,6 +76,17 @@ public final class Reflector {
7376
*/
7477
public Reflector(URLClassLoader isolatedClassLoader) {
7578
this.isolatedClassLoader = isolatedClassLoader;
79+
// Best-effort cleanup: close classloader when Reflector is GC'd.
80+
// Under mvnd, abandoned Reflectors from previous builds leak
81+
// URLClassLoader handles until GC collects them.
82+
URLClassLoader cl = isolatedClassLoader;
83+
CLEANER.register(this, () -> {
84+
try {
85+
cl.close();
86+
} catch (IOException e) {
87+
// ignore
88+
}
89+
});
7690
}
7791

7892
private Reflector(URLClassLoader isolatedClassLoader, Object classFinder,
@@ -167,6 +181,19 @@ public URL getResource(String name) {
167181
return isolatedClassLoader.getResource(name);
168182
}
169183

184+
/**
185+
* Closes the isolated class loader, releasing file handles on JARs.
186+
* Idempotent: safe to call multiple times.
187+
*/
188+
@Override
189+
public void close() {
190+
try {
191+
isolatedClassLoader.close();
192+
} catch (IOException e) {
193+
// ignore
194+
}
195+
}
196+
170197
/**
171198
* Creates a copy of the given Flow mojo, loading classes the isolated
172199
* classloader.
@@ -396,7 +423,7 @@ public URL getResource(String name) {
396423
if (url == null) {
397424
url = ClassLoader.getPlatformClassLoader().getResource(name);
398425
}
399-
return url;
426+
return ReflectionsClassFinder.disableJarCaching(url);
400427
}
401428

402429
@Override
@@ -406,13 +433,15 @@ public Enumeration<URL> getResources(String name) throws IOException {
406433
// Collect resources from all classloaders
407434
Enumeration<URL> resources = super.getResources(name);
408435
while (resources.hasMoreElements()) {
409-
allResources.add(resources.nextElement());
436+
allResources.add(ReflectionsClassFinder
437+
.disableJarCaching(resources.nextElement()));
410438
}
411439

412440
if (delegate != null) {
413441
resources = delegate.getResources(name);
414442
while (resources.hasMoreElements()) {
415-
URL url = resources.nextElement();
443+
URL url = ReflectionsClassFinder
444+
.disableJarCaching(resources.nextElement());
416445
if (!allResources.contains(url)) {
417446
allResources.add(url);
418447
}
@@ -421,7 +450,8 @@ public Enumeration<URL> getResources(String name) throws IOException {
421450

422451
resources = ClassLoader.getPlatformClassLoader().getResources(name);
423452
while (resources.hasMoreElements()) {
424-
URL url = resources.nextElement();
453+
URL url = ReflectionsClassFinder
454+
.disableJarCaching(resources.nextElement());
425455
if (!allResources.contains(url)) {
426456
allResources.add(url);
427457
}

‎flow-plugins/flow-maven-plugin/src/main/java/com/vaadin/flow/plugin/maven/FlowModeAbstractMojo.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,10 @@ public boolean isHillaAvailable() {
430430
* @return true if Hilla is available, false otherwise
431431
*/
432432
public static boolean isHillaAvailable(MavenProject mavenProject) {
433-
return Reflector.of(mavenProject, null, null).getResource(
434-
"com/vaadin/hilla/EndpointController.class") != null;
433+
try (Reflector reflector = Reflector.of(mavenProject, null, null)) {
434+
return reflector.getResource(
435+
"com/vaadin/hilla/EndpointController.class") != null;
436+
}
435437
}
436438

437439
/**

‎flow-plugins/flow-maven-plugin/src/main/java/com/vaadin/flow/plugin/maven/Reflector.java‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@
1515
*/
1616
package com.vaadin.flow.plugin.maven;
1717

18+
import java.io.Closeable;
1819
import java.io.File;
1920
import java.io.IOException;
2021
import java.lang.annotation.Documented;
2122
import java.lang.annotation.ElementType;
2223
import java.lang.annotation.Retention;
2324
import java.lang.annotation.RetentionPolicy;
2425
import java.lang.annotation.Target;
26+
import java.lang.ref.Cleaner;
2527
import java.lang.reflect.Field;
2628
import java.lang.reflect.Modifier;
2729
import java.net.URL;
@@ -61,7 +63,7 @@
6163
/**
6264
* Helper class to deal with classloading of Flow plugin mojos.
6365
*/
64-
public final class Reflector {
66+
public final class Reflector implements Closeable {
6567

6668
public static final String INCLUDE_FROM_COMPILE_DEPS_REGEX = ".*(/|\\\\)(portlet-api|javax\\.servlet-api)-.+jar$";
6769
private static final Set<String> DEPENDENCIES_GROUP_EXCLUSIONS = Set.of(
@@ -73,6 +75,7 @@ public final class Reflector {
7375
"org.zeroturnaround:zt-exec:jar");
7476
private static final ScopeArtifactFilter PRODUCTION_SCOPE_FILTER = new ScopeArtifactFilter(
7577
Artifact.SCOPE_COMPILE_PLUS_RUNTIME);
78+
private static final Cleaner CLEANER = Cleaner.create();
7679
private static final Logger log = LoggerFactory.getLogger(Reflector.class);
7780

7881
private final URLClassLoader isolatedClassLoader;
@@ -87,6 +90,17 @@ public final class Reflector {
8790
*/
8891
Reflector(URLClassLoader isolatedClassLoader) {
8992
this.isolatedClassLoader = isolatedClassLoader;
93+
// Best-effort cleanup: close classloader when Reflector is GC'd.
94+
// Under mvnd, abandoned Reflectors from previous builds leak
95+
// URLClassLoader handles until GC collects them.
96+
URLClassLoader cl = isolatedClassLoader;
97+
CLEANER.register(this, () -> {
98+
try {
99+
cl.close();
100+
} catch (IOException e) {
101+
// ignore
102+
}
103+
});
90104
}
91105

92106
private Reflector(URLClassLoader isolatedClassLoader, Object classFinder,
@@ -181,6 +195,19 @@ public URL getResource(String name) {
181195
return isolatedClassLoader.getResource(name);
182196
}
183197

198+
/**
199+
* Closes the isolated class loader, releasing file handles on JARs.
200+
* Idempotent: safe to call multiple times.
201+
*/
202+
@Override
203+
public void close() {
204+
try {
205+
isolatedClassLoader.close();
206+
} catch (IOException e) {
207+
log.debug("Error closing isolated class loader", e);
208+
}
209+
}
210+
184211
/**
185212
* Creates a copy of the given Flow mojo, loading classes the isolated
186213
* classloader.
@@ -468,7 +495,7 @@ public URL getResource(String name) {
468495
if (url == null) {
469496
url = ClassLoader.getPlatformClassLoader().getResource(name);
470497
}
471-
return url;
498+
return ReflectionsClassFinder.disableJarCaching(url);
472499
}
473500

474501
@Override
@@ -478,13 +505,15 @@ public Enumeration<URL> getResources(String name) throws IOException {
478505
// Collect resources from all classloaders
479506
Enumeration<URL> resources = super.getResources(name);
480507
while (resources.hasMoreElements()) {
481-
allResources.add(resources.nextElement());
508+
allResources.add(ReflectionsClassFinder
509+
.disableJarCaching(resources.nextElement()));
482510
}
483511

484512
if (delegate != null) {
485513
resources = delegate.getResources(name);
486514
while (resources.hasMoreElements()) {
487-
URL url = resources.nextElement();
515+
URL url = ReflectionsClassFinder
516+
.disableJarCaching(resources.nextElement());
488517
if (!allResources.contains(url)) {
489518
allResources.add(url);
490519
}
@@ -493,7 +522,8 @@ public Enumeration<URL> getResources(String name) throws IOException {
493522

494523
resources = ClassLoader.getPlatformClassLoader().getResources(name);
495524
while (resources.hasMoreElements()) {
496-
URL url = resources.nextElement();
525+
URL url = ReflectionsClassFinder
526+
.disableJarCaching(resources.nextElement());
497527
if (!allResources.contains(url)) {
498528
allResources.add(url);
499529
}

‎flow-plugins/flow-maven-plugin/src/test/java/com/vaadin/flow/plugin/maven/ReflectorTest.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.io.File;
2121
import java.net.URL;
2222
import java.net.URLClassLoader;
23+
import java.net.URLConnection;
2324
import java.nio.file.Path;
2425
import java.util.Arrays;
2526
import java.util.HashSet;
@@ -296,6 +297,55 @@ public void reflector_frontendScannerConfig_vaadinArtifactAlwaysIncluded()
296297
expectedArtifacts);
297298
}
298299

300+
@Test
301+
public void close_isIdempotent() {
302+
reflector.close();
303+
reflector.close(); // second close should not throw
304+
}
305+
306+
@Test
307+
public void getResource_jarUrlDisablesCaching() throws Exception {
308+
// The reflector's isolated classloader delegates to maven.api realm
309+
// which contains the test JAR
310+
MavenProject project = new MavenProject();
311+
project.setGroupId("com.vaadin.test");
312+
project.setArtifactId("reflector-tests");
313+
project.setBuild(new Build());
314+
project.getBuild().setOutputDirectory(PROJECT_TARGET_FOLDER);
315+
project.setArtifacts(Set.of());
316+
317+
MojoExecution exec = new MojoExecution(new MojoDescriptor());
318+
PluginDescriptor pluginDescriptor = new PluginDescriptor();
319+
exec.getMojoDescriptor().setPluginDescriptor(pluginDescriptor);
320+
pluginDescriptor.setGroupId("com.vaadin.test");
321+
pluginDescriptor.setArtifactId("test-plugin");
322+
pluginDescriptor.setArtifacts(List.of());
323+
ClassWorld classWorld = new ClassWorld("maven.api", null);
324+
classWorld.getRealm("maven.api")
325+
.addURL(Path
326+
.of("src", "test", "resources",
327+
"jar-without-frontend-resources.jar")
328+
.toUri().toURL());
329+
pluginDescriptor.setClassRealm(classWorld.newRealm("maven-plugin"));
330+
331+
Reflector execReflector = Reflector.of(project, exec, null);
332+
try {
333+
URL resource = execReflector.getIsolatedClassLoader()
334+
.getResource("org/json/CookieList.class");
335+
Assert.assertNotNull("Resource should be found in JAR", resource);
336+
Assert.assertEquals("jar", resource.getProtocol());
337+
338+
URLConnection conn = resource.openConnection();
339+
Assert.assertFalse(
340+
"jar: URL connections should have caching disabled "
341+
+ "to prevent stale JarFileFactory entries "
342+
+ "under mvnd",
343+
conn.getUseCaches());
344+
} finally {
345+
execReflector.close();
346+
}
347+
}
348+
299349
@Test
300350
public void reflector_disabledFrontendScannerConfig_getsFullIsolatedClassLoader()
301351
throws Exception {

‎flow-plugins/flow-plugin-base/src/main/java/com/vaadin/flow/server/scanner/ReflectionsClassFinder.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,23 @@ private Set<Class<?>> getAnnotatedByRepeatedAnnotation(
119119

120120
@Override
121121
public URL getResource(String name) {
122-
URL url = classLoader.getResource(name);
122+
return disableJarCaching(classLoader.getResource(name));
123+
}
124+
125+
/**
126+
* Wraps a {@code jar:} URL with a handler that disables JVM-level JAR
127+
* caching. Prevents {@code JarFileFactory} from caching stale
128+
* {@code JarFile} instances across daemon builds (Gradle daemon, mvnd).
129+
*
130+
* @param url
131+
* the URL to wrap, may be {@code null}
132+
* @return a wrapped URL with caching disabled for {@code jar:} protocol, or
133+
* the original URL for other protocols or {@code null} input
134+
*/
135+
public static URL disableJarCaching(URL url) {
123136
if (url == null || !"jar".equals(url.getProtocol())) {
124137
return url;
125138
}
126-
// Wrap jar: URLs with a handler that disables JVM-level JAR caching.
127-
// Without this, JarFileFactory keeps a static cache of JarFile
128-
// instances that become stale when JARs are rewritten between Gradle
129-
// daemon builds, causing ZipException ("invalid LOC header").
130139
try {
131140
return new URL(null, url.toExternalForm(), new URLStreamHandler() {
132141
@Override

0 commit comments

Comments
 (0)