8338281: jshell does not run shutdown hooks

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

@@ -173,7 +173,7 @@ private boolean processCommand() throws IOException {
                     } catch (Throwable ex) {
                         // JShell-core not waiting for a result, ignore
                     }
-                    return true;
+                    return false;
                 }
                 default: {
                     Object arg = in.readObject();

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

@@ -48,6 +48,7 @@
 import com.sun.jdi.ThreadReference;
 import com.sun.jdi.VMDisconnectedException;
 import com.sun.jdi.VirtualMachine;
+import java.util.concurrent.TimeUnit;
 import jdk.jshell.spi.ExecutionControl;
 import jdk.jshell.spi.ExecutionEnv;
 import static jdk.jshell.execution.Util.remoteInputOutput;
@@ -65,6 +66,8 @@
  */
 public class JdiDefaultExecutionControl extends JdiExecutionControl {
 
+    private static final int SHUTDOWN_TIMEOUT = 1; //1 second
+
     private VirtualMachine vm;
     private Process process;
     private final String remoteAgent;
@@ -218,6 +221,20 @@ public void stop() throws EngineTerminationException, InternalException {
     @Override
     public void close() {
         super.close();
+
+        Process remoteProcess;
+
+        synchronized (this) {
+            remoteProcess = this.process;
+        }
+
+        if (remoteProcess != null) {
+            try {
+                remoteProcess.waitFor(SHUTDOWN_TIMEOUT, TimeUnit.SECONDS);
+            } catch (InterruptedException ex) {
+                debug(ex, "waitFor remote");
+            }
+        }
         disposeVM();
     }
 

test/langtools/jdk/jshell/ShutdownTest.java

@@ -28,15 +28,21 @@
  * @run testng ShutdownTest
  */
 
+import java.io.IOException;
+import java.lang.reflect.Method;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.function.Consumer;
 
 import jdk.jshell.JShell;
 import jdk.jshell.JShell.Subscription;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import org.testng.annotations.BeforeMethod;
 
-@Test
 public class ShutdownTest extends KullaTesting {
 
     int shutdownCount;
@@ -53,13 +59,15 @@ public void testExit() {
         assertEquals(shutdownCount, 1);
     }
 
+    @Test
     public void testCloseCallback() {
         shutdownCount = 0;
         getState().onShutdown(this::shutdownCounter);
         getState().close();
         assertEquals(shutdownCount, 1);
     }
 
+    @Test
     public void testCloseUnsubscribe() {
         shutdownCount = 0;
         Subscription token = getState().onShutdown(this::shutdownCounter);
@@ -68,6 +76,7 @@ public void testCloseUnsubscribe() {
         assertEquals(shutdownCount, 0);
     }
 
+    @Test
     public void testTwoShutdownListeners() {
         ShutdownListener listener1 = new ShutdownListener();
         ShutdownListener listener2 = new ShutdownListener();
@@ -118,6 +127,51 @@ public void testSubscriptionAfterShutdown() {
         getState().onShutdown(e -> {});
     }
 
+    @Test
+    public void testRunShutdownHooks() throws IOException {
+        Path temporary = Paths.get("temp");
+        Files.newOutputStream(temporary).close();
+        assertEval("import java.io.*;");
+        assertEval("import java.nio.file.*;");
+        assertEval("""
+                        Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+                            try {
+                                Files.delete(Paths.get("$TEMPORARY"));
+                            } catch (IOException ex) {
+                                //ignored
+                            }
+                        }))
+                        """.replace("$TEMPORARY", temporary.toAbsolutePath()
+                                                           .toString()
+                                                           .replace("\\", "\\\\")));
+        getState().close();
+        assertFalse(Files.exists(temporary));
+    }
+
+    private Method currentTestMethod;
+
+    @BeforeMethod
+    public void setUp(Method testMethod) {
+        currentTestMethod = testMethod;
+        super.setUp();
+    }
+
+    @BeforeMethod
+    public void setUp() {
+    }
+
+    @Override
+    public void setUp(Consumer<JShell.Builder> bc) {
+        Consumer<JShell.Builder> augmentedBuilder = switch (currentTestMethod.getName()) {
+            case "testRunShutdownHooks" -> builder -> {
+                builder.executionEngine(Presets.TEST_STANDARD_EXECUTION);
+                bc.accept(builder);
+            };
+            default -> bc;
+        };
+        super.setUp(augmentedBuilder);
+    }
+
     private static class ShutdownListener implements Consumer<JShell> {
         private int count;