Ensure JDI test VM cleanup after failed startup

org.eclipse.jdt.debug.jdi.tests/tests/org/eclipse/debug/jdi/tests/AbstractJDITest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2024 IBM Corporation and others.
+ * Copyright (c) 2000, 2026 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -10,6 +10,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Carsten Hammer - cleanup after failed test VM startup
  *******************************************************************************/
 package org.eclipse.debug.jdi.tests;
 
@@ -24,9 +25,11 @@
 import java.util.Map;
 import java.util.StringTokenizer;
 import java.util.Vector;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.debug.jdi.tests.program.MainClass;
 import org.eclipse.jdi.Bootstrap;
+import org.eclipse.jdi.TimeoutException;
 import org.eclipse.jdi.internal.VirtualMachineImpl;
 
 import com.sun.jdi.AbsentInformationException;
@@ -671,8 +674,13 @@ private boolean isExcludedTest(String testName) {
 	 * Launches the target VM and connects to VM.
 	 */
 	protected void launchTargetAndConnectToVM() {
-		launchTarget();
-		connectToVM();
+		try {
+			launchTarget();
+			connectToVM();
+		} catch (RuntimeException | Error failure) {
+			cleanupAfterFailedStartup(failure);
+			throw failure;
+		}
 	}
 
 	protected boolean vmIsRunning() {
@@ -912,9 +920,15 @@ protected void connectToVM() {
 		startConsoleReaders();
 
 
-		// Contact the VM (try at least 10 times for 5 seconds)
+		// Bound each attach as well as the retry loop. A zero connector timeout
+		// waits forever if a peer accepts the socket but does not complete JDWP.
+		long timeoutNanos = TimeUnit.SECONDS.toNanos(5);
+		Throwable connectionFailure = null;
 		long n0 = System.nanoTime();
 		for (int i = 0; i < 10000; i++) {
+			if (Thread.currentThread().isInterrupted()) {
+				throw new Error("Interrupted while connecting to the VM", new InterruptedException());
+			}
 			try {
 				VirtualMachineManager manager = Bootstrap.virtualMachineManager();
 				List<AttachingConnector> connectors = manager.attachingConnectors();
@@ -925,6 +939,8 @@ protected void connectToVM() {
 				Map<String, Argument> args = connector.defaultArguments();
 				args.get("port").setValue(String.valueOf(fBackEndPort));
 				args.get("hostname").setValue("localhost");
+				long remainingMillis = Math.max(1, TimeUnit.NANOSECONDS.toMillis(timeoutNanos - (System.nanoTime() - n0)));
+				args.get("timeout").setValue(Long.toString(remainingMillis));
 
 				fVM = connector.attach(args);
 				if (fVMTraceFlags != com.sun.jdi.VirtualMachine.TRACE_NONE) {
@@ -933,16 +949,19 @@ protected void connectToVM() {
 				break;
 			} catch (IllegalConnectorArgumentsException e) {
 				e.printStackTrace();
-			} catch (IOException e) {
+			} catch (IOException | TimeoutException e) {
+				connectionFailure = e;
 				long n1 = System.nanoTime();
-				if (i > 10 && n1 - n0 > 5_000_000_000L) {
+				if (n1 - n0 >= timeoutNanos) {
 					e.printStackTrace();
 					System.out.println("Could not contact the VM at localhost" + ":" + fBackEndPort + " after " + (n1 - n0) / 1_000_000L + "ms");
 					break;
 				}
 				try {
 					Thread.sleep(10);
 				} catch (InterruptedException interrupted) {
+					Thread.currentThread().interrupt();
+					throw new Error("Interrupted while connecting to the VM", interrupted);
 				}
 			}
 		}
@@ -963,15 +982,36 @@ protected void connectToVM() {
 				} catch (IOException e) {
 				}
 
-				// Shut it down
-				killVM();
 			}
-			throw new Error("Could not contact the VM");
+			Error failure = new Error("Could not contact the VM", connectionFailure);
+			// Retain pre-cleanup state in the test failure even when console output
+			// from the target process is missing from the CI test report.
+			failure.addSuppressed(new IllegalStateException("JDI startup at localhost:" + fBackEndPort
+					+ "; target " + describeProcess(fLaunchedVM) + "; proxy " + describeProcess(fLaunchedProxy)
+					+ "; runtime=" + Runtime.version() + "; vendor=" + System.getProperty("java.vendor")));
+			throw failure;
 		}
 		long n1 = System.nanoTime(); // for example after ~ 110ms
 		System.out.println("Connected to localhost" + ":" + fBackEndPort + " after " + (n1 - n0) / 1_000_000L + "ms");
 		startEventReader();
 	}
+
+	private static String describeProcess(Process process) {
+		if (process == null) {
+			return "not started";
+		}
+		String pid;
+		try {
+			pid = Long.toString(process.pid());
+		} catch (UnsupportedOperationException e) {
+			pid = "unavailable";
+		}
+		try {
+			return "pid=" + pid + ", alive=false, exitCode=" + process.exitValue();
+		} catch (IllegalThreadStateException e) {
+			return "pid=" + pid + ", alive=true";
+		}
+	}
 	/**
 	 * Initializes the fields that are used by this test only.
 	 */
@@ -1158,7 +1198,23 @@ public void setInControl(boolean inControl) {
 	 */
 	protected void launchTargetAndStartProgram() {
 		launchTargetAndConnectToVM();
-		startProgram();
+		try {
+			startProgram();
+		} catch (RuntimeException | Error failure) {
+			cleanupAfterFailedStartup(failure);
+			throw failure;
+		}
+	}
+
+	private void cleanupAfterFailedStartup(Throwable failure) {
+		// JUnit does not call tearDown() when setUp() fails.
+		try {
+			shutDownTarget();
+		} catch (RuntimeException | Error cleanupFailure) {
+			if (cleanupFailure != failure) {
+				failure.addSuppressed(cleanupFailure);
+			}
+		}
 	}
 	/**
 	 * Init tests
@@ -1201,29 +1257,57 @@ protected void stopReaders() {
 	 * Shut down the target.
 	 */
 	public void shutDownTarget() {
-		stopReaders();
-		if (fVM != null) {
+		boolean hadTarget = fVM != null || fLaunchedVM != null || fLaunchedProxy != null;
+		Throwable failure = null;
+		try {
+			stopReaders();
+			if (fVM != null) {
+				try {
+					fVM.exit(0);
+				} catch (VMDisconnectedException e) {
+				}
+			}
+		} catch (RuntimeException | Error e) {
+			failure = e;
+			throw e;
+		} finally {
 			try {
-				fVM.exit(0);
-			} catch (VMDisconnectedException e) {
+				killVM();
+			} catch (RuntimeException | Error cleanupFailure) {
+				if (failure == null) {
+					throw cleanupFailure;
+				}
+				if (failure != cleanupFailure) {
+					failure.addSuppressed(cleanupFailure);
+				}
+			} finally {
+				fVM = null;
+				// Keep a handle to a process whose termination failed.
+				if (fLaunchedVM != null && !fLaunchedVM.isAlive()) {
+					fLaunchedVM = null;
+				}
+				if (fLaunchedProxy != null && !fLaunchedProxy.isAlive()) {
+					fLaunchedProxy = null;
+				}
+				fEventReader = null;
+				fConsoleReader = null;
+				fConsoleErrorReader = null;
+				fProxyReader = null;
+				fProxyErrorReader = null;
+				if (hadTarget) {
+					selectNextPort();
+				}
 			}
 		}
+	}
 
-		fVM = null;
-		fLaunchedVM = null;
-
-		// We want subsequent connections to use different ports, unless a
-		// VM exec sting is given.
+	private static void selectNextPort() {
+		// Preserve the port embedded in a user-supplied VM command.
 		if (fVmCmd == null) {
-			ServerSocket socket;
-			try {
-				socket = new ServerSocket(0);
-				int availablePort = socket.getLocalPort();
-				socket.close(); //Available but always initialized
-				fBackEndPort = availablePort;
+			try (ServerSocket socket = new ServerSocket(0)) {
+				fBackEndPort = socket.getLocalPort();
 			} catch (IOException e) {
-				fBackEndPort +=2;
-
+				fBackEndPort += 2;
 			}
 		}
 	}
@@ -1317,15 +1401,12 @@ private void startEventReader() {
 	 * Stops the event reader.
 	 */
 	private void stopEventReader() {
-		fEventReader.stop();
+		if (fEventReader != null) {
+			fEventReader.stop();
+		}
 	}
 	protected void killVM() {
-		if (fLaunchedVM != null) {
-			fLaunchedVM.destroy();
-		}
-		if (fLaunchedProxy != null) {
-			fLaunchedProxy.destroy();
-		}
+		TestProcessCleanup.terminate(fLaunchedVM, fLaunchedProxy);
 	}
 	/**
 	 * Starts the target program.

org.eclipse.jdt.debug.jdi.tests/tests/org/eclipse/debug/jdi/tests/TestProcessCleanup.java

@@ -0,0 +1,96 @@
+/*******************************************************************************
+ * Copyright (c) 2026 Carsten Hammer and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Carsten Hammer - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.debug.jdi.tests;
+
+import java.util.concurrent.TimeUnit;
+
+/** Bounded termination of processes owned by a JDI test. */
+final class TestProcessCleanup {
+	private static final long EXIT_TIMEOUT_MILLIS = 5000;
+
+	private TestProcessCleanup() {
+	}
+
+	static void terminate(Process... processes) {
+		Throwable failure = null;
+		for (Process process : processes) {
+			try {
+				terminate(process, EXIT_TIMEOUT_MILLIS);
+			} catch (RuntimeException | Error e) {
+				if (failure == null) {
+					failure = e;
+				} else if (failure != e) {
+					failure.addSuppressed(e);
+				}
+			}
+		}
+		if (failure instanceof RuntimeException e) {
+			throw e;
+		}
+		if (failure instanceof Error e) {
+			throw e;
+		}
+	}
+
+	private static void terminate(Process process, long timeoutMillis) {
+		if (process == null || !process.isAlive()) {
+			return;
+		}
+		// Cleanup must still run when startup was interrupted. Restore the flag below.
+		boolean interrupted = Thread.interrupted();
+		try {
+			process.destroy();
+			if (!interrupted) {
+				try {
+					if (process.waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) {
+						return;
+					}
+				} catch (InterruptedException e) {
+					interrupted = true;
+				}
+			}
+			process.destroyForcibly();
+			// Wait despite interruption without extending the deadline, then
+			// restore the interrupt status below.
+			long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeoutMillis);
+			while (process.isAlive()) {
+				long remaining = deadline - System.nanoTime();
+				if (remaining <= 0) {
+					throw terminationFailure(process);
+				}
+				try {
+					if (!process.waitFor(remaining, TimeUnit.NANOSECONDS) && process.isAlive()) {
+						throw terminationFailure(process);
+					}
+				} catch (InterruptedException e) {
+					interrupted = true;
+				}
+			}
+		} finally {
+			if (interrupted) {
+				Thread.currentThread().interrupt();
+			}
+		}
+	}
+
+	private static IllegalStateException terminationFailure(Process process) {
+		String pid;
+		try {
+			pid = Long.toString(process.pid());
+		} catch (UnsupportedOperationException e) {
+			pid = "unavailable";
+		}
+		return new IllegalStateException("Test process " + pid + " did not terminate after destroyForcibly()");
+	}
+}