Issue #11909 - duplicate --modules=<name> can trigger ConcurrentModif…

…icationException + Reworked tracking of enabled modules to not trigger change in "sources" Set if the module is already enabled.
jetty · Jun 12, 2024 · a616aca · a616aca
1 parent 8e6ab93
commit a616aca
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 4 deletions.
diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java
@@ -1426,9 +1426,11 @@ private void selectModules(String source, List<String> moduleNames)
     {
         for (String moduleName : moduleNames)
         {
-            modules.add(moduleName);
-            Set<String> set = sources.computeIfAbsent(moduleName, k -> new HashSet<>());
-            set.add(source);
+            if (modules.add(moduleName))
+            {
+                Set<String> set = sources.computeIfAbsent(moduleName, k -> new HashSet<>());
+                set.add(source);
+            }
         }
     }
 

diff --git a/jetty-core/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java b/jetty-core/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java
@@ -30,6 +30,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -183,4 +184,27 @@ public void testJvmArgExpansion() throws Exception
         );
         assertThat(commandLine, containsString(expectedExpansion));
     }
+
+    @Test
+    public void testModulesDeclaredTwice() throws Exception
+    {
+        List<String> cmdLineArgs = new ArrayList<>();
+
+        Path homePath = MavenPaths.findTestResourceDir("dist-home");
+        Path basePath = MavenPaths.findTestResourceDir("overdeclared-modules");
+        cmdLineArgs.add("jetty.home=" + homePath);
+        cmdLineArgs.add("user.dir=" + basePath);
+
+        Main main = new Main();
+
+        cmdLineArgs.add("--module=main");
+
+        // The "main" module is enabled in both ...
+        // 1) overdeclared-modules/start.d/config.ini
+        // 2) command-line
+        // This shouldn't result in an error
+        StartArgs args = main.processCommandLine(cmdLineArgs.toArray(new String[0]));
+
+        assertThat(args.getSelectedModules(), hasItem("main"));
+    }
 }
diff --git a/jetty-core/jetty-start/src/test/resources/dist-home/start.ini b/jetty-core/jetty-start/src/test/resources/dist-home/start.ini
diff --git a/...re/jetty-start/src/test/resources/overdeclared-modules/resources/jetty-logging.properties b/...re/jetty-start/src/test/resources/overdeclared-modules/resources/jetty-logging.properties
diff --git a/jetty-core/jetty-start/src/test/resources/overdeclared-modules/start.d/config.ini b/jetty-core/jetty-start/src/test/resources/overdeclared-modules/start.d/config.ini
@@ -0,0 +1 @@
+--modules=main