8319311: JShell Process Builder should be configurable

Reviewed-by: asotona

src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,7 +33,6 @@ import java.net.InetAddress;
 import java.net.ServerSocket;
 import java.net.Socket;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -48,9 +47,11 @@ import com.sun.jdi.StackFrame;
 import com.sun.jdi.ThreadReference;
 import com.sun.jdi.VMDisconnectedException;
 import com.sun.jdi.VirtualMachine;
+import java.io.PrintStream;
 import java.util.Optional;
 import java.util.stream.Stream;
 import jdk.jshell.JShellConsole;
+import jdk.jshell.execution.JdiDefaultExecutionControl.JdiStarter.TargetDescription;
 import jdk.jshell.spi.ExecutionControl;
 import jdk.jshell.spi.ExecutionEnv;
 import static jdk.jshell.execution.Util.remoteInputOutput;
@@ -94,8 +95,7 @@ public class JdiDefaultExecutionControl extends JdiExecutionControl {
      * @return the channel
      * @throws IOException if there are errors in set-up
      */
-    static ExecutionControl create(ExecutionEnv env, String remoteAgent,
+    static ExecutionControl create(ExecutionEnv env, Map<String, String> parameters, String remoteAgent, int timeout, JdiStarter starter) throws IOException {
-            boolean isLaunch, String host, int timeout) throws IOException {
         try (final ServerSocket listener = new ServerSocket(0, 1, InetAddress.getLoopbackAddress())) {
             // timeout on I/O-socket
             listener.setSoTimeout(timeout);
@@ -107,13 +107,37 @@ public class JdiDefaultExecutionControl extends JdiExecutionControl {
                                   //disable System.console():
                                   List.of("-Djdk.console=" + consoleModule).stream())
                           .toList();
+            ExecutionEnv augmentedEnv = new ExecutionEnv() {
+                @Override
+                public InputStream userIn() {
+                    return env.userIn();
+                }
+
+                @Override
+                public PrintStream userOut() {
+                    return env.userOut();
+                }
+
+                @Override
+                public PrintStream userErr() {
+                    return env.userErr();
+                }
+
+                @Override
+                public List<String> extraRemoteVMOptions() {
+                    return augmentedremoteVMOptions;
+                }
+
+                @Override
+                public void closeDown() {
+                    env.closeDown();
+                }
+            };
 
             // Set-up the JDI connection
-            JdiInitiator jdii = new JdiInitiator(port,
+            TargetDescription target = starter.start(augmentedEnv, parameters, port);
-                    augmentedremoteVMOptions, remoteAgent, isLaunch, host,
+            VirtualMachine vm = target.vm();
-                    timeout, Collections.emptyMap());
+            Process process = target.process();
-            VirtualMachine vm = jdii.vm();
-            Process process = jdii.process();
 
             List<Consumer<String>> deathListeners = new ArrayList<>();
             Util.detectJdiExitEvent(vm, s -> {
@@ -294,4 +318,31 @@ public class JdiDefaultExecutionControl extends JdiExecutionControl {
         // Reserved for future logging
     }
 
+    /**
+     * Start an external process where the user's snippets can be run.
+     *
+     * @since 22
+     */
+    public interface JdiStarter {
+        /**
+         * Start the external process based on the given parameters. The external
+         * process should connect to the given {@code port} to communicate with the
+         * driving instance of JShell.
+         *
+         * @param env the execution context
+         * @param parameters additional execution parameters
+         * @param port the port to which the remote process should connect
+         * @return a description of the started external process
+         * @throws RuntimeException if the process cannot be started
+         * @throws Error if the process cannot be started
+         */
+        public TargetDescription start(ExecutionEnv env, Map<String, String> parameters, int port);
+
+        /**
+         * The description of a started external process.
+         * @param vm the JDI's {@code VirtualMachine}
+         * @param process the external {@code Process}
+         */
+        public record TargetDescription(VirtualMachine vm, Process process) {}
+    }
 }

src/jdk.jshell/share/classes/jdk/jshell/execution/JdiExecutionControlProvider.java

@@ -26,9 +26,11 @@
 package jdk.jshell.execution;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import jdk.jshell.execution.JdiDefaultExecutionControl.JdiStarter;
 import jdk.jshell.spi.ExecutionControl;
 import jdk.jshell.spi.ExecutionControlProvider;
 import jdk.jshell.spi.ExecutionEnv;
@@ -66,6 +68,8 @@ public class JdiExecutionControlProvider implements ExecutionControlProvider {
      */
     private static final int DEFAULT_TIMEOUT = 5000;
 
+    private final JdiStarter starter;
+
     /**
      * Create an instance.  An instance can be used to
      * {@linkplain  #generate generate} an {@link ExecutionControl} instance
@@ -73,6 +77,37 @@ public class JdiExecutionControlProvider implements ExecutionControlProvider {
      * process.
      */
     public JdiExecutionControlProvider() {
+        this(new JdiStarter() {
+            @Override
+            public TargetDescription start(ExecutionEnv env, Map<String, String> parameters, int port) {
+                String remoteAgent = parameters.get(PARAM_REMOTE_AGENT);
+                int timeout = Integer.parseUnsignedInt(
+                    parameters.get(PARAM_TIMEOUT));
+                String host = parameters.get(PARAM_HOST_NAME);
+                String sIsLaunch = parameters.get(PARAM_LAUNCH)
+                                             .toLowerCase(Locale.ROOT);
+                boolean isLaunch = sIsLaunch.length() > 0
+                    && ("true".startsWith(sIsLaunch) || "yes".startsWith(sIsLaunch));
+
+                JdiInitiator jdii = new JdiInitiator(port,
+                    env.extraRemoteVMOptions(), remoteAgent, isLaunch, host,
+                    timeout, Collections.emptyMap());
+                return new TargetDescription(jdii.vm(), jdii.process());
+            }
+        });
+    }
+
+    /**
+     * Create an instance.  An instance can be used to
+     * {@linkplain  #generate generate} an {@link ExecutionControl} instance
+     * that uses the Java Debug Interface as part of the control of a remote
+     * process. The provided {@code start} will be used to start the remote process.
+     *
+     * @param starter starter that will create the remote process
+     * @since 22
+     */
+    public JdiExecutionControlProvider(JdiStarter starter) {
+        this.starter = starter;
     }
 
     /**
@@ -142,14 +177,14 @@ public class JdiExecutionControlProvider implements ExecutionControlProvider {
         if (parameters == null) {
             parameters = dp;
         }
-        String remoteAgent = parameters.getOrDefault(PARAM_REMOTE_AGENT, dp.get(PARAM_REMOTE_AGENT));
+        parameters = new HashMap<>(parameters);
+        String remoteAgent = parameters.computeIfAbsent(PARAM_REMOTE_AGENT, x -> dp.get(PARAM_REMOTE_AGENT));
         int timeout = Integer.parseUnsignedInt(
-                parameters.getOrDefault(PARAM_TIMEOUT, dp.get(PARAM_TIMEOUT)));
+                parameters.computeIfAbsent(PARAM_TIMEOUT, x -> dp.get(PARAM_TIMEOUT)));
-        String host = parameters.getOrDefault(PARAM_HOST_NAME, dp.get(PARAM_HOST_NAME));
+        parameters.putIfAbsent(PARAM_HOST_NAME, dp.get(PARAM_HOST_NAME));
-        String sIsLaunch = parameters.getOrDefault(PARAM_LAUNCH, dp.get(PARAM_LAUNCH)).toLowerCase(Locale.ROOT);
+        parameters.putIfAbsent(PARAM_LAUNCH, dp.get(PARAM_LAUNCH));
-        boolean isLaunch = sIsLaunch.length() > 0
+
-                && ("true".startsWith(sIsLaunch) || "yes".startsWith(sIsLaunch));
+        return JdiDefaultExecutionControl.create(env, parameters, remoteAgent, timeout, starter);
-        return JdiDefaultExecutionControl.create(env, remoteAgent, isLaunch, host, timeout);
     }
 
 }

src/jdk.jshell/share/classes/jdk/jshell/execution/JdiInitiator.java

@@ -153,14 +153,55 @@ public class JdiInitiator {
      */
     private VirtualMachine listenTarget(int port, List<String> remoteVMOptions) {
         ListeningConnector listener = (ListeningConnector) connector;
+        try {
+            String addr;
+
+            try {
+                // Start listening, get the JDI connection address
+                addr = listener.startListening(connectorArgs);
+                debug("Listening at address: " + addr);
+            } catch (Throwable t) {
+                throw reportLaunchFail(t, "listen");
+            }
+
+            runListenProcess(addr, port, remoteVMOptions, process -> {
+                // Accept the connection from the remote agent
+                vm = timedVirtualMachineCreation(() -> listener.accept(connectorArgs),
+                        () -> process.waitFor());
+                try {
+                    listener.stopListening(connectorArgs);
+                } catch (IOException | IllegalConnectorArgumentsException ex) {
+                    // ignore
+                }
+            });
+            return vm;
+        } catch (Throwable ex) {
+            try {
+                listener.stopListening(connectorArgs);
+            } catch (IOException | IllegalConnectorArgumentsException iex) {
+                // ignore
+            }
+            throw ex;
+        }
+    }
+
+    /**
+     * Create a process that will attach to the given address.
+     * @param jdiAddress address on which a JDI server is waiting for a connection
+     * @param jshellControlPort the port which the remote agent should connect to
+     * @param remoteVMOptions VM options for the remote agent VM
+     * @param setupVM a callback that should be called then the remote agent process
+     *                is created. The callback will setup the JDI's {@code VirtualMachine}.
+     * @since 22
+     */
+    protected void runListenProcess(String jdiAddress,
+                                    int jshellControlPort,
+                                    List<String> remoteVMOptions,
+                                    ProcessStarted setupVM) {
         // Files to collection to output of a start-up failure
         File crashErrorFile = createTempFile("error");
         File crashOutputFile = createTempFile("output");
         try {
-            // Start listening, get the JDI connection address
-            String addr = listener.startListening(connectorArgs);
-            debug("Listening at address: " + addr);
-
             // Launch the RemoteAgent requesting a connection on that address
             String javaHome = System.getProperty("java.home");
             List<String> args = new ArrayList<>();
@@ -168,35 +209,23 @@ public class JdiInitiator {
                     ? "java"
                     : javaHome + File.separator + "bin" + File.separator + "java");
             args.add("-agentlib:jdwp=transport=" + connector.transport().name() +
-                    ",address=" + addr);
+                    ",address=" + jdiAddress);
             args.addAll(remoteVMOptions);
             args.add(remoteAgent);
-            args.add("" + port);
+            args.add("" + jshellControlPort);
             ProcessBuilder pb = new ProcessBuilder(args);
             pb.redirectError(crashErrorFile);
             pb.redirectOutput(crashOutputFile);
             process = pb.start();
 
-            // Accept the connection from the remote agent
+            setupVM.processStarted(process);
-            vm = timedVirtualMachineCreation(() -> listener.accept(connectorArgs),
+
-                    () -> process.waitFor());
-            try {
-                listener.stopListening(connectorArgs);
-            } catch (IOException | IllegalConnectorArgumentsException ex) {
-                // ignore
-            }
             crashErrorFile.delete();
             crashOutputFile.delete();
-            return vm;
         } catch (Throwable ex) {
             if (process != null) {
                 process.destroyForcibly();
             }
-            try {
-                listener.stopListening(connectorArgs);
-            } catch (IOException | IllegalConnectorArgumentsException iex) {
-                // ignore
-            }
             String text = readFile(crashErrorFile) + readFile(crashOutputFile);
             crashErrorFile.delete();
             crashOutputFile.delete();
@@ -328,4 +357,19 @@ public class JdiInitiator {
         // Reserved for future logging
     }
 
+    /**
+     * Callback that should invoked when the remote process is invoked.
+     *
+     * @since 22
+     */
+    protected interface ProcessStarted {
+        /**
+         * Notify the process has been started.
+         *
+         * @param p the {@code Process}
+         * @throws Throwable thrown when anything goes wrong.
+         */
+        public void processStarted(Process p) throws Throwable;
+    }
+
 }

test/langtools/jdk/jshell/JdiStarterTest.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8319311
+ * @summary Tests JdiStarter
+ * @modules jdk.jshell/jdk.jshell jdk.jshell/jdk.jshell.spi jdk.jshell/jdk.jshell.execution
+ * @run testng JdiStarterTest
+ */
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import org.testng.annotations.Test;
+import jdk.jshell.JShell;
+import jdk.jshell.SnippetEvent;
+import jdk.jshell.execution.JdiDefaultExecutionControl.JdiStarter;
+import jdk.jshell.execution.JdiDefaultExecutionControl.JdiStarter.TargetDescription;
+import jdk.jshell.execution.JdiExecutionControlProvider;
+import jdk.jshell.execution.JdiInitiator;
+import static org.testng.Assert.assertEquals;
+
+@Test
+public class JdiStarterTest {
+
+    public void jdiStarter() {
+        // turn on logging of launch failures
+        Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
+        JdiStarter starter = (env, parameters, port) -> {
+            assertEquals(parameters.get(JdiExecutionControlProvider.PARAM_HOST_NAME), "");
+            assertEquals(parameters.get(JdiExecutionControlProvider.PARAM_LAUNCH), "false");
+            assertEquals(parameters.get(JdiExecutionControlProvider.PARAM_REMOTE_AGENT), "jdk.jshell.execution.RemoteExecutionControl");
+            assertEquals(parameters.get(JdiExecutionControlProvider.PARAM_TIMEOUT), "5000");
+            JdiInitiator jdii =
+                    new JdiInitiator(port,
+                                     env.extraRemoteVMOptions(),
+                                     "jdk.jshell.execution.RemoteExecutionControl",
+                                     false,
+                                     null,
+                                     5000,
+                                     Collections.emptyMap());
+            return new TargetDescription(jdii.vm(), jdii.process());
+        };
+        JShell jshell =
+                JShell.builder()
+                      .executionEngine(new JdiExecutionControlProvider(starter), Map.of())
+                      .build();
+        List<SnippetEvent> evts = jshell.eval("1 + 2");
+        assertEquals(1, evts.size());
+        assertEquals("3", evts.get(0).value());
+    }
+}