From a87b8dfeb48e715722e671312a73dc3ba802f33e Mon Sep 17 00:00:00 2001 From: Satyen Subramaniam Date: Wed, 14 May 2025 18:54:49 +0000 Subject: [PATCH 1/2] Backport 8c8b5801fd9d28a71edf3bd8d1fae857817e27de --- .../execution/ExecutionControlForwarder.java | 2 +- .../execution/JdiDefaultExecutionControl.java | 18 ++++++ test/langtools/jdk/jshell/ShutdownTest.java | 56 ++++++++++++++++++- 3 files changed, 74 insertions(+), 2 deletions(-) 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 061e548878b..5b23a3f1f92 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java +++ b/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(); 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 c4c2ff09259..d3752383481 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java @@ -48,6 +48,8 @@ import com.sun.jdi.ThreadReference; import com.sun.jdi.VMDisconnectedException; import com.sun.jdi.VirtualMachine; +import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; import jdk.jshell.spi.ExecutionControl; import jdk.jshell.spi.ExecutionEnv; import static jdk.jshell.execution.Util.remoteInputOutput; @@ -65,6 +67,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 +222,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(); } 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 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 void testExit() { assertEquals(shutdownCount, 1); } + @Test public void testCloseCallback() { shutdownCount = 0; getState().onShutdown(this::shutdownCounter); @@ -60,6 +67,7 @@ public void testCloseCallback() { 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 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; From 4ea314142e4ed974b00aea2b45efdc1db619dd64 Mon Sep 17 00:00:00 2001 From: Satyen Subramaniam Date: Thu, 22 May 2025 19:26:01 +0000 Subject: [PATCH 2/2] Removing java.util.stream.Stream; --- .../classes/jdk/jshell/execution/JdiDefaultExecutionControl.java | 1 - 1 file changed, 1 deletion(-) 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 d3752383481..2dffd90a6fe 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java @@ -49,7 +49,6 @@ import com.sun.jdi.VMDisconnectedException; import com.sun.jdi.VirtualMachine; import java.util.concurrent.TimeUnit; -import java.util.stream.Stream; import jdk.jshell.spi.ExecutionControl; import jdk.jshell.spi.ExecutionEnv; import static jdk.jshell.execution.Util.remoteInputOutput;