Issue #6476 - warn on exec (#6597)

Fixes #6476 - Show message if JVM args are present but new JVM is spawned

* Improved documentation by correctly redacting out `jetty-halt.xml`,
an XML file that is only necessary for rendering the documentation.
* Added WARN message when new JVM is spawned.
* Updated documentation.
* Updated --list-config to report whether a JVM is forked.
* Added test case.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

documentation/jetty-asciidoctor-extensions/src/main/java/org/eclipse/jetty/docs/JettyIncludeExtension.java

@@ -144,6 +144,7 @@ private String captureOutput(Document document, Map<String, Object> attributes,
                 .map(line -> redact(line, run.getConfig().getMavenLocalRepository(), "/path/to/maven.repository"))
                 .map(line -> redact(line, run.getConfig().getJettyHome().toString(), "/path/to/jetty.home"))
                 .map(line -> redact(line, run.getConfig().getJettyBase().toString(), "/path/to/jetty.base"))
+                .map(line -> regexpRedact(line, "(^| )[^ ]+/etc/jetty-halt\\.xml", ""))
                 .map(line -> redact(line, (String)document.getAttribute("project-version"), (String)document.getAttribute("version")));
             lines = replace(lines, (String)attributes.get("replace"));
             lines = delete(lines, (String)attributes.get("delete"));
@@ -160,6 +161,13 @@ private String redact(String line, String target, String replacement)
             return line;
         }
 
+        private String regexpRedact(String line, String regexp, String replacement)
+        {
+            if (regexp != null && replacement != null)
+                return line.replaceAll(regexp, replacement);
+            return line;
+        }
+
         private Stream<String> replace(Stream<String> lines, String replace)
         {
             if (replace == null)
@@ -178,8 +186,7 @@ private Stream<String> delete(Stream<String> lines, String delete)
             if (delete == null)
                 return lines;
             Pattern regExp = Pattern.compile(delete);
-            return lines.filter(line -> !regExp.matcher(line).find())
-                .filter(line -> !line.contains("jetty-halt.xml"));
+            return lines.filter(line -> !regExp.matcher(line).find());
         }
 
         private Stream<String> denoteLineStart(Stream<String> lines)

documentation/jetty-documentation/src/main/asciidoc/operations-guide/modules/modules.adoc

@@ -396,6 +396,8 @@ When the `[exec]` section is present, the JVM running the Jetty start mechanism
 This is necessary because JVM options such as `-Xmx` (that specifies the max JVM heap size) cannot be changed in a running JVM.
 For an example, see xref:og-start-configure-custom-module-exec[this section].
 
+You can avoid that the Jetty start mechanism forks the second JVM, as explained in xref:og-start-configure-dry-run[this section].
+
 [[og-modules-directive-jpms]]
 ===== [jpms]
 

documentation/jetty-documentation/src/main/asciidoc/operations-guide/start/start-configure.adoc

@@ -163,10 +163,15 @@ $ java -jar $JETTY_HOME/start.jar --add-modules=jvm
 
 Since the module defines an `[exec]` section, it will fork _another_ JVM when Jetty is started.
 
-This means that when you start Jetty, there will be _two_ JVMs running: one spawned by you when you run `java -jar $JETTY_HOME/start.jar`, and another spawned by the Jetty start mechanism with the JVM options you specified (that cannot be applied to an already running JVM).
+This means that when you start Jetty, there will be _two_ JVMs running: one created by you when you run `java -jar $JETTY_HOME/start.jar`, and another forked by the Jetty start mechanism with the JVM options you specified (that cannot be applied to an already running JVM).
 
 Again, you can xref:og-start-configure-dry-run[display the JVM command line] to verify that it is correct.
 
+[TIP]
+====
+The second JVM forked by the Jetty start mechanism when one of the modules requires forking, for example a module that contains an `[exec]` section, may not be desirable, and may be avoided as explained in xref:og-start-configure-dry-run[this section].
+====
+
 [[og-start-configure-display]]
 ===== Displaying the Configuration
 
@@ -205,7 +210,28 @@ Some option, such as `--jpms`, imply `--exec`, as it won't be possible to modify
 
 To start Jetty without forking a second JVM, the `--dry-run` option can be used to generate a command line that is then executed so that starting Jetty only spawns one JVM.
 
-The `--dry-run` option is quite flexible and below you can find a few examples of how to use it to generate scripts or to create an arguments file that can be passed to the `java` executable.
+IMPORTANT: You can use the `--dry-run` option as explained below to avoid forking a second JVM when using modules that have the `[exec]` section, or the `--exec` option, or when using the `--jpms` option.
+
+For example, using the `--dry-run` option with the `jvm.mod` introduced in xref:og-start-configure-custom-module-exec[this section] produces the following command line:
+
+----
+$ java -jar $JETTY_HOME/start.jar --dry-run
+----
+
+[source,options=nowrap]
+----
+include::jetty[setupModules="src/main/asciidoc/operations-guide/start/jvm.mod",setupArgs="--add-modules=http,jvm",args="--dry-run",replace="( ),$1\\\n"]
+----
+
+You can then run the generated command line.
+
+For example, in the Linux `bash` shell you can run it by wrapping it into `$(\...)`:
+
+----
+$ $(java -jar $JETTY_HOME/start.jar --dry-run)
+----
+
+The `--dry-run` option is quite flexible and below you can find a few examples of how to use it to avoid forking a second JVM, or generating scripts or creating an arguments file that can be passed to (a possibly alternative) `java` executable.
 
 To display the `java` executable used to start Jetty:
 
@@ -304,7 +330,13 @@ $ java -jar $JETTY_HOME/start.jar --dry-run=##opts,path,main,args## > /tmp/jvm_c
 $ /some/other/java @/tmp/jvm_cmd_line.txt
 ----
 
-Alternatively, they can be combined in a shell script:
+Using `--dry-run=opts,path,main,args` can be used to avoid that the Jetty start mechanism forks a second JVM when using modules that require forking:
+
+----
+$ java $(java -jar $JETTY_HOME/start.jar --dry-run=opts,path,main,args)
+----
+
+The output of different `--dry-run` executions can be creatively combined in a shell script:
 
 [source,subs=quotes]
 ----

jetty-start/src/main/java/org/eclipse/jetty/start/Main.java

@@ -30,6 +30,7 @@
 import java.nio.file.Path;
 import java.util.List;
 import java.util.Locale;
+import java.util.stream.Collectors;
 
 import org.eclipse.jetty.start.Props.Prop;
 import org.eclipse.jetty.start.config.CommandLineConfigSource;
@@ -467,6 +468,15 @@ else if (args.isCreateFiles() || !args.getStartModules().isEmpty())
         {
             CommandLineBuilder cmd = args.getMainArgs(StartArgs.ALL_PARTS);
             cmd.debug();
+
+            List<String> execModules = args.getEnabledModules().stream()
+                .map(name -> args.getAllModules().get(name))
+                // Keep only the forking modules.
+                .filter(module -> !module.getJvmArgs().isEmpty())
+                .map(Module::getName)
+                .collect(Collectors.toList());
+            StartLog.warn("Forking second JVM due to forking module(s): %s. Use --dry-run to generate the command line to avoid forking.", execModules);
+
             ProcessBuilder pbuilder = new ProcessBuilder(cmd.getArgs());
             StartLog.endStartLog();
             final Process process = pbuilder.start();

jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java

@@ -302,7 +302,7 @@ public void dumpEnvironment()
         // Jetty Environment
         System.out.println();
         System.out.println("Jetty Environment:");
-        System.out.println("-----------------");
+        System.out.println("------------------");
         dumpProperty(JETTY_VERSION_KEY);
         dumpProperty(JETTY_TAG_NAME_KEY);
         dumpProperty(JETTY_BUILDNUM_KEY);
@@ -330,26 +330,20 @@ public void dumpEnvironment()
 
     public void dumpJvmArgs()
     {
-        System.out.println();
-        System.out.println("JVM Arguments:");
-        System.out.println("--------------");
         if (jvmArgs.isEmpty())
-        {
-            System.out.println(" (no jvm args specified)");
             return;
-        }
+
+        System.out.println();
+        System.out.println("Forked JVM Arguments:");
+        System.out.println("---------------------");
 
         for (String jvmArgKey : jvmArgs)
         {
             String value = System.getProperty(jvmArgKey);
             if (value != null)
-            {
                 System.out.printf(" %s = %s%n", jvmArgKey, value);
-            }
             else
-            {
                 System.out.printf(" %s%n", jvmArgKey);
-            }
         }
     }
 

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java

@@ -931,4 +931,37 @@ public void testUnixDomain() throws Exception
             }
         }
     }
+
+    @Test
+    public void testModuleWithExecEmitsWarning() throws Exception
+    {
+        String jettyVersion = System.getProperty("jettyVersion");
+        JettyHomeTester distribution = JettyHomeTester.Builder.newInstance()
+            .jettyVersion(jettyVersion)
+            .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+            .build();
+
+        Path jettyBase = distribution.getJettyBase();
+        Path jettyBaseModules = jettyBase.resolve("modules");
+        Files.createDirectories(jettyBaseModules);
+        Path execModule = jettyBaseModules.resolve("exec.mod");
+        String module = "" +
+            "[exec]\n" +
+            "--show-version";
+        Files.write(execModule, List.of(module), StandardOpenOption.CREATE);
+
+        try (JettyHomeTester.Run run1 = distribution.start(List.of("--add-modules=http,exec")))
+        {
+            assertTrue(run1.awaitFor(10, TimeUnit.SECONDS));
+            assertEquals(0, run1.getExitValue());
+
+            int port = distribution.freePort();
+            try (JettyHomeTester.Run run2 = distribution.start("jetty.http.port=" + port))
+            {
+                assertTrue(run2.awaitConsoleLogsFor("Started Server@", 10, TimeUnit.SECONDS));
+                assertTrue(run2.getLogs().stream()
+                    .anyMatch(log -> log.contains("WARN") && log.contains("Forking")));
+            }
+        }
+    }
 }