From 27966555321266744140fc8d04dd5804cfa5a6a0 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Fri, 13 Mar 2026 14:03:56 +1000 Subject: [PATCH 1/4] Avoid some non thread safety issue with local repo here using system properties, add a way to configure the local repo via a new builder Signed-off-by: Olivier Lamy --- .../maven/shared/verifier/Verifier.java | 71 +++++++++++++++++-- 1 file changed, 67 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/maven/shared/verifier/Verifier.java b/src/main/java/org/apache/maven/shared/verifier/Verifier.java index a7ac3a0..e9cf481 100644 --- a/src/main/java/org/apache/maven/shared/verifier/Verifier.java +++ b/src/main/java/org/apache/maven/shared/verifier/Verifier.java @@ -327,6 +327,62 @@ public Verifier(String basedir, String settingsFile, boolean debug, String maven setMavenHome(mavenHome); } + /** + * Builder for creating {@link Verifier} instances with explicit configuration. + *

+ * When {@link #localRepository(String)} is set, the local repository is pre-populated + * before {@code findLocalRepo()} runs, so the system property + * {@code maven.repo.local} is never consulted. This eliminates the race condition that + * occurs when multiple {@code Verifier} instances are constructed concurrently in the + * same JVM while {@link Embedded3xLauncher#run} is manipulating the JVM-global system + * properties table ({@code System.setProperties(null)}). + */ + public static class Builder { + private final String basedir; + private String settingsFile; + private String localRepository; + + public Builder(String basedir) { + this.basedir = basedir; + } + + public Builder settingsFile(String settingsFile) { + this.settingsFile = settingsFile; + return this; + } + + /** + * Sets the local repository path explicitly. + * When provided, the system property {@code maven.repo.local} and the settings file + * are both bypassed for local repository resolution, making construction race-free. + */ + public Builder localRepository(String localRepository) { + this.localRepository = localRepository; + return this; + } + + public Verifier build() throws VerificationException { + return new Verifier(this); + } + } + + private Verifier(Builder builder) throws VerificationException { + this.basedir = builder.basedir; + this.forkMode = System.getProperty("verifier.forkMode"); + // Pre-set localRepo before findLocalRepo() so that the settings-file and + // system-property lookups in findLocalRepo() are both skipped when an + // explicit value has been provided via the builder. + this.localRepo = builder.localRepository; + if (builder.settingsFile != null) { + this.settingsFile = builder.settingsFile; + } + findLocalRepo(this.settingsFile); + this.mavenHome = System.getProperty("maven.home"); + setForkMode(); + useWrapper = Files.exists(Paths.get(getBasedir(), "mvnw")); + this.defaultCliArguments = DEFAULT_CLI_ARGUMENTS.clone(); + } + public void setLocalRepo(String localRepo) { this.localRepo = localRepo; } @@ -1272,10 +1328,7 @@ private static String getLogContents(File logFile) { private void findLocalRepo(String settingsFile) throws VerificationException { if (localRepo == null) { - localRepo = System.getProperty("maven.repo.local"); - } - - if (localRepo == null) { + // Settings file is the primary configuration source — consult it first. try { localRepo = retrieveLocalRepo(settingsFile); } catch (SettingsBuildingException e) { @@ -1283,6 +1336,16 @@ private void findLocalRepo(String settingsFile) throws VerificationException { } } + if (localRepo == null) { + // System property is the last resort before the hardcoded default. + // It is intentionally checked AFTER the settings file because + // Embedded3xLauncher.run() calls System.setProperties(null) during embedded + // Maven executions, creating a race window where this property may transiently + // hold a value injected by a concurrent Maven invocation running in the same JVM. + // Prefer Builder.localRepository() for an entirely race-free alternative. + localRepo = System.getProperty("maven.repo.local"); + } + if (localRepo == null) { localRepo = USER_HOME + "/.m2/repository"; } From 657c669eff2646c41d351dd7058a3a868ff1520b Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Sat, 14 Mar 2026 09:47:50 +1000 Subject: [PATCH 2/4] Improve builder with more field to configure, add mvnExecutable full feature Signed-off-by: Olivier Lamy --- .../maven/shared/verifier/ForkedLauncher.java | 42 +++-- .../maven/shared/verifier/Verifier.java | 145 +++++++++++------- 2 files changed, 114 insertions(+), 73 deletions(-) diff --git a/src/main/java/org/apache/maven/shared/verifier/ForkedLauncher.java b/src/main/java/org/apache/maven/shared/verifier/ForkedLauncher.java index b5afd80..c82e430 100644 --- a/src/main/java/org/apache/maven/shared/verifier/ForkedLauncher.java +++ b/src/main/java/org/apache/maven/shared/verifier/ForkedLauncher.java @@ -67,29 +67,37 @@ class ForkedLauncher implements MavenLauncher { } ForkedLauncher(String mavenHome, Map envVars, boolean debugJvm, boolean wrapper) { + this(mavenHome, envVars, debugJvm, wrapper, null); + } + + ForkedLauncher( + String mavenHome, Map envVars, boolean debugJvm, boolean wrapper, String mvnExecutable) { this.mavenHome = mavenHome; this.envVars = envVars; + if (mvnExecutable != null) { + executable = mvnExecutable; + } else { + if (wrapper) { + final StringBuilder script = new StringBuilder(); - if (wrapper) { - final StringBuilder script = new StringBuilder(); - - if (!isWindows()) { - script.append("./"); - } - - script.append("mvnw"); + if (!isWindows()) { + script.append("./"); + } - if (debugJvm) { - script.append("Debug"); - } - executable = script.toString(); - } else { - String script = "mvn" + (debugJvm ? "Debug" : ""); + script.append("mvnw"); - if (mavenHome != null) { - executable = new File(mavenHome, "bin/" + script).getPath(); + if (debugJvm) { + script.append("Debug"); + } + executable = script.toString(); } else { - executable = script; + String script = "mvn" + (debugJvm ? "Debug" : ""); + + if (mavenHome != null) { + executable = new File(mavenHome, "bin/" + script).getPath(); + } else { + executable = script; + } } } } diff --git a/src/main/java/org/apache/maven/shared/verifier/Verifier.java b/src/main/java/org/apache/maven/shared/verifier/Verifier.java index e9cf481..5377b70 100644 --- a/src/main/java/org/apache/maven/shared/verifier/Verifier.java +++ b/src/main/java/org/apache/maven/shared/verifier/Verifier.java @@ -172,6 +172,8 @@ public class Verifier { private String mavenHome; + private String mvnExecutable; + // will launch mvn with -X private boolean mavenDebug = false; @@ -327,62 +329,6 @@ public Verifier(String basedir, String settingsFile, boolean debug, String maven setMavenHome(mavenHome); } - /** - * Builder for creating {@link Verifier} instances with explicit configuration. - *

- * When {@link #localRepository(String)} is set, the local repository is pre-populated - * before {@code findLocalRepo()} runs, so the system property - * {@code maven.repo.local} is never consulted. This eliminates the race condition that - * occurs when multiple {@code Verifier} instances are constructed concurrently in the - * same JVM while {@link Embedded3xLauncher#run} is manipulating the JVM-global system - * properties table ({@code System.setProperties(null)}). - */ - public static class Builder { - private final String basedir; - private String settingsFile; - private String localRepository; - - public Builder(String basedir) { - this.basedir = basedir; - } - - public Builder settingsFile(String settingsFile) { - this.settingsFile = settingsFile; - return this; - } - - /** - * Sets the local repository path explicitly. - * When provided, the system property {@code maven.repo.local} and the settings file - * are both bypassed for local repository resolution, making construction race-free. - */ - public Builder localRepository(String localRepository) { - this.localRepository = localRepository; - return this; - } - - public Verifier build() throws VerificationException { - return new Verifier(this); - } - } - - private Verifier(Builder builder) throws VerificationException { - this.basedir = builder.basedir; - this.forkMode = System.getProperty("verifier.forkMode"); - // Pre-set localRepo before findLocalRepo() so that the settings-file and - // system-property lookups in findLocalRepo() are both skipped when an - // explicit value has been provided via the builder. - this.localRepo = builder.localRepository; - if (builder.settingsFile != null) { - this.settingsFile = builder.settingsFile; - } - findLocalRepo(this.settingsFile); - this.mavenHome = System.getProperty("maven.home"); - setForkMode(); - useWrapper = Files.exists(Paths.get(getBasedir(), "mvnw")); - this.defaultCliArguments = DEFAULT_CLI_ARGUMENTS.clone(); - } - public void setLocalRepo(String localRepo) { this.localRepo = localRepo; } @@ -1682,4 +1628,91 @@ private void setForkMode() { forkMode = "auto"; } } + + /** + * Builder for creating {@link Verifier} instances with explicit configuration. + *

+ * When {@link #localRepository(String)} is set, the local repository is pre-populated + * before {@code findLocalRepo()} runs, so the system property + * {@code maven.repo.local} is never consulted. This eliminates the race condition that + * occurs when multiple {@code Verifier} instances are constructed concurrently in the + * same JVM while {@link Embedded3xLauncher#run} is manipulating the JVM-global system + * properties table ({@code System.setProperties(null)}). + */ + public static class Builder { + private final String basedir; + private String settingsFile; + private String localRepository; + private String forkMode; + private Boolean forkJvm; + private String mvnExecutable; + + public Builder(String basedir) { + this.basedir = basedir; + } + + public Builder settingsFile(String settingsFile) { + this.settingsFile = settingsFile; + return this; + } + + /** + * Sets the local repository path explicitly. + * When provided, the system property {@code maven.repo.local} and the settings file + * are both bypassed for local repository resolution, making construction race-free. + */ + public Builder localRepository(String localRepository) { + this.localRepository = localRepository; + return this; + } + + /** + * Sets the forkMode explicitly. + * When provided, the system property verifier.forkMode is bypassed. + */ + public Builder forkMode(String forkMode) { + this.forkMode = forkMode; + return this; + } + + /** + * Sets the forkJvm explicitly. + */ + public Builder forkJvm(Boolean forkJvm) { + this.forkJvm = forkJvm; + return this; + } + + /** + * Sets the maven full path executable explicitly. + * When provided, the maven executable is used directly without any lookup or resolution. + */ + public Builder mvnExecutable(String mvnExecutable) { + this.mvnExecutable = mvnExecutable; + return this; + } + + public Verifier build() throws VerificationException { + return new Verifier(this); + } + } + + private Verifier(Builder builder) throws VerificationException { + this.basedir = builder.basedir; + this.forkMode = builder.forkMode == null ? System.getProperty("verifier.forkMode") : builder.forkMode; + setForkMode(); + this.forkJvm = builder.forkJvm; + // Pre-set localRepo before findLocalRepo() so that the settings-file and + // system-property lookups in findLocalRepo() are both skipped when an + // explicit value has been provided via the builder. + this.localRepo = builder.localRepository; + if (builder.settingsFile != null) { + this.settingsFile = builder.settingsFile; + } + findLocalRepo(this.settingsFile); + this.mavenHome = System.getProperty("maven.home"); + this.useWrapper = Files.exists(Paths.get(getBasedir(), "mvnw")); + this.defaultCliArguments = DEFAULT_CLI_ARGUMENTS.clone(); + this.mvnExecutable = builder.mvnExecutable; + } } From 79f3684de158c03e274268417d4438528cc4b3fc Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Sat, 14 Mar 2026 10:34:57 +1000 Subject: [PATCH 3/4] more configuration via the Builder Signed-off-by: Olivier Lamy --- .../org/apache/maven/shared/verifier/Verifier.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/maven/shared/verifier/Verifier.java b/src/main/java/org/apache/maven/shared/verifier/Verifier.java index 5377b70..e6b9946 100644 --- a/src/main/java/org/apache/maven/shared/verifier/Verifier.java +++ b/src/main/java/org/apache/maven/shared/verifier/Verifier.java @@ -1222,7 +1222,7 @@ protected MavenLauncher getMavenLauncher(Map envVars) throws Lau return embeddedLauncher; } else { - return new ForkedLauncher(mavenHome, envVars, debugJvm, useWrapper); + return new ForkedLauncher(mavenHome, envVars, debugJvm, useWrapper, mvnExecutable); } } @@ -1646,6 +1646,7 @@ public static class Builder { private String forkMode; private Boolean forkJvm; private String mvnExecutable; + private String[] defaultCliArguments; public Builder(String basedir) { this.basedir = basedir; @@ -1692,6 +1693,14 @@ public Builder mvnExecutable(String mvnExecutable) { return this; } + /** + * Sets the default CLI arguments to be used for every execution + */ + public Builder defaultCliArguments(String[] defaultCliArguments) { + this.defaultCliArguments = defaultCliArguments; + return this; + } + public Verifier build() throws VerificationException { return new Verifier(this); } @@ -1712,7 +1721,8 @@ private Verifier(Builder builder) throws VerificationException { findLocalRepo(this.settingsFile); this.mavenHome = System.getProperty("maven.home"); this.useWrapper = Files.exists(Paths.get(getBasedir(), "mvnw")); - this.defaultCliArguments = DEFAULT_CLI_ARGUMENTS.clone(); + this.defaultCliArguments = + builder.defaultCliArguments == null ? DEFAULT_CLI_ARGUMENTS.clone() : builder.defaultCliArguments; this.mvnExecutable = builder.mvnExecutable; } } From db5153c12c48790b9a23b4d13a6b658190ddcf6b Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Sat, 14 Mar 2026 14:19:33 +1000 Subject: [PATCH 4/4] cannot be empty Signed-off-by: Olivier Lamy --- .../org/apache/maven/shared/verifier/ForkedLauncher.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/maven/shared/verifier/ForkedLauncher.java b/src/main/java/org/apache/maven/shared/verifier/ForkedLauncher.java index c82e430..a5226bc 100644 --- a/src/main/java/org/apache/maven/shared/verifier/ForkedLauncher.java +++ b/src/main/java/org/apache/maven/shared/verifier/ForkedLauncher.java @@ -74,7 +74,7 @@ class ForkedLauncher implements MavenLauncher { String mavenHome, Map envVars, boolean debugJvm, boolean wrapper, String mvnExecutable) { this.mavenHome = mavenHome; this.envVars = envVars; - if (mvnExecutable != null) { + if (mvnExecutable != null && !mvnExecutable.isEmpty()) { executable = mvnExecutable; } else { if (wrapper) { @@ -127,10 +127,8 @@ public int run( cmd.setWorkingDirectory(workingDirectory); - for (Object o : systemProperties.keySet()) { - String key = (String) o; - String value = systemProperties.getProperty(key); - cmd.createArg().setValue("-D" + key + "=" + value); + for (Map.Entry entry : systemProperties.entrySet()) { + cmd.createArg().setValue("-D" + entry.getKey() + "=" + entry.getValue()); } for (String cliArg : cliArgs) {