diff --git a/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java b/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java index fdb126f93ec..54c4131ee3b 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java @@ -177,7 +177,7 @@ class ExecutionControlForwarder { } catch (Throwable ex) { // JShell-core not waiting for a result, ignore } - return true; + return false; } default: { Object arg = in.readObject(); diff --git a/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java b/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java index 2d429691b60..889b8494bab 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java @@ -49,6 +49,7 @@ import com.sun.jdi.VMDisconnectedException; import com.sun.jdi.VirtualMachine; import java.io.PrintStream; import java.util.Optional; +import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import jdk.jshell.JShellConsole; import jdk.jshell.execution.JdiDefaultExecutionControl.JdiStarter.TargetDescription; @@ -70,6 +71,8 @@ import jdk.jshell.execution.impl.ConsoleImpl.ConsoleOutputStream; */ public class JdiDefaultExecutionControl extends JdiExecutionControl { + private static final int SHUTDOWN_TIMEOUT = 1; //1 second + private VirtualMachine vm; private Process process; private final String remoteAgent; @@ -267,6 +270,20 @@ public class JdiDefaultExecutionControl extends JdiExecutionControl { @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(); } diff --git a/test/langtools/jdk/jshell/ShutdownTest.java b/test/langtools/jdk/jshell/ShutdownTest.java index 45431f66162..da4e516f94c 100644 --- a/test/langtools/jdk/jshell/ShutdownTest.java +++ b/test/langtools/jdk/jshell/ShutdownTest.java @@ -28,6 +28,11 @@ * @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; @@ -35,8 +40,9 @@ 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,6 +59,7 @@ public class ShutdownTest extends KullaTesting { assertEquals(shutdownCount, 1); } + @Test public void testCloseCallback() { shutdownCount = 0; getState().onShutdown(this::shutdownCounter); @@ -60,6 +67,7 @@ public class ShutdownTest extends KullaTesting { assertEquals(shutdownCount, 1); } + @Test public void testCloseUnsubscribe() { shutdownCount = 0; Subscription token = getState().onShutdown(this::shutdownCounter); @@ -68,6 +76,7 @@ public class ShutdownTest extends KullaTesting { assertEquals(shutdownCount, 0); } + @Test public void testTwoShutdownListeners() { ShutdownListener listener1 = new ShutdownListener(); ShutdownListener listener2 = new ShutdownListener(); @@ -118,6 +127,51 @@ public class ShutdownTest extends KullaTesting { 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 bc) { + Consumer 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 { private int count;