Ensure JDI test VM cleanup after failed startup

[carstenartur]

What it does

Make the shared JDI test harness clean up owned VM/proxy processes after
failed launch, attach or program startup, and prevent a stalled JDWP
handshake from blocking the attach operation indefinitely.

The changes are confined to org.eclipse.jdt.debug.jdi.tests. They
affect the test harness, not product-side debugger connection settings
or breakpoint behavior.

Related: #990.
This infrastructure change is reviewed separately from the
breakpoint-removal fix.

Why the shared infrastructure needs to change

Failed startup can bypass normal teardown

The harness can create processes and readers before VM startup has
completed. With the JUnit TestCase lifecycle used here, an exception
from setUp() prevents tearDown() from running: setUp() is called
before the try/finally that invokes teardown.

See JUnit's TestCase.runBare() implementation.

Consequently, cleanup cannot rely exclusively on normal test teardown.
It must also run when the startup operation itself fails.

The existing shutdown path additionally assumes that the event reader
has already been created. That assumption does not hold when attaching
to the VM fails. Process destruction also previously returned without
waiting for process termination, so requesting cleanup did not establish
that the owned process had actually exited.

The cleanup belongs at the shared startup entry points rather than in
individual tests. This covers both the usual launch-and-start path and
callers of launchTargetAndConnectToVM(), including VirtualMachineTest,
without duplicating failure handling across test classes.

A retry deadline does not bound a blocked attach call

The existing connection loop checks elapsed time between failed attach
attempts, but does not supply a timeout to the connector. A peer that
accepts the socket without completing the JDWP handshake can therefore
keep an individual attach attempt blocked, preventing both the retry
check and startup cleanup from being reached.

Cleanup alone cannot address that case. The connector call also needs
a finite timeout.

These are specific lifecycle and timeout defects. This PR does not
claim that they explain every earlier CI startup failure, or that
earlier connection-refused failures had the same cause as the
deliberately simulated silent-peer scenario.

Implementation

Exception-safe startup and shutdown

launchTargetAndConnectToVM() and launchTargetAndStartProgram() invoke
cleanup when their launch, attach or program-start operations throw a
RuntimeException or Error. The original failure is rethrown, and a
cleanup failure is attached as a suppressed exception rather than
replacing it.

Shutdown tolerates an absent event reader and attempts owned-process
cleanup in a finally block, including when stopping readers or
requesting VM exit throws.

After cleanup, VM and reader references are cleared. Handles to
processes that are still alive are retained rather than discarded.
Repeated shutdown after successful cleanup does not terminate the
processes again or select another port.

The existing automatic port-selection mechanism is reused after
cleanup when target state was present. A user-supplied VM command keeps
its configured port.

Bounded process termination

The package-private TestProcessCleanup helper centralizes termination
of processes owned by the test harness.

For each live process, it requests normal termination and waits up to
five seconds. If necessary, it requests forced termination and waits
with a separate five-second deadline. Missing or already exited
processes are ignored.

Interruption does not abandon cleanup: the helper attempts forced
termination and restores the interrupt flag afterward. A failure for
one process does not prevent attempts to terminate the remaining
processes; additional failures are retained as suppressed exceptions.

A process that remains alive after forced termination produces an
IllegalStateException. Diagnostic formatting tolerates
Process.pid() throwing UnsupportedOperationException, reporting
the PID as unavailable without masking the intended cleanup failure.

Keeping this logic in a small helper allows the termination sequence,
failure aggregation and interrupt handling to be tested using process
test doubles, without depending on an overloaded or unresponsive JVM.

Attach timeout and diagnostic retention

The connection loop passes its remaining nominal five-second budget
to the connector instead of leaving the connector timeout unbounded.
Transport timeouts participate in the connection-failure handling.
Interruption detected before an attach attempt or during the retry
sleep aborts startup while preserving the interrupt flag.

When connection attempts fail, the reported error retains the last
connection failure as its cause. Additional diagnostics record the
port, VM/proxy process state and test-runtime information before
cleanup changes that state.

Behavioral impact and boundaries

This is a shared test-harness behavior change, not merely additional
regression tests.

The attach policy is stricter than before: an attempt that previously
blocked beyond the nominal retry budget can now fail with a timeout.
A slow test VM that previously connected only after exceeding that
budget may therefore fail sooner.

Normal test shutdown also changes. Owned-process termination is now
attempted after the VM exit request, and shutdown may wait for process
exit or escalate to forced termination instead of returning immediately.
Failure to terminate an owned process is reported rather than silently
ignored.

The process timeouts apply separately to each termination phase and
each process. They are not a single five-second deadline for the whole
shutdown operation, nor does this PR establish an end-to-end deadline
for every startup or shutdown action.

Reader shutdown continues to use the existing reader stop mechanism.
This change adds null-safe invocation and reference cleanup; it does
not introduce reader-thread joins or redesign reader synchronization.

The startup cleanup covers the launch, attach and program-start paths
described above. It is not a general recovery mechanism for arbitrary
failures in every test-specific setup hook.

No product code, test exclusions or existing test assertions are
changed.

How to test

Run org.eclipse.debug.jdi.tests.AutomatedSuite using the repository's
existing JDI test configuration. Both new test classes are registered
in that suite, which also performs the shared harness initialization.

The current change adds 20 regression tests.

VMStartupCleanupTest — 18 tests

The lifecycle tests inject failures during launch, attach and program
startup. They exercise the JUnit runBare() path and verify that
cleanup occurs even though the fixture's normal tearDown() is not
called after failed setup.

Coverage includes cleanup before the event reader exists, reader stop
requests and reference clearing, preservation of the original startup
failure, automatic/custom port handling, repeated shutdown, and a
subsequent startup after successful cleanup.

Process test doubles cover normal and forced termination, bounded
waits, interruption before or during cleanup, missing/already exited
processes, continued cleanup after another process fails, and
diagnostics when a PID is unavailable.

A separate test starts a real Java subprocess, waits until it signals
readiness, injects an attach failure, and verifies that startup cleanup
terminates the subprocess. This supplements the deterministic
test-double coverage with an actual process-lifecycle check.

VMConnectionTimeoutTest — 2 tests

These tests use the real JDI connector and a local socket peer that
accepts the connection and reads the JDWP handshake without replying.

They verify that the connection attempt fails within the test's
bounded wait, retains the transport timeout and pre-cleanup diagnostics,
and reaches owned-process cleanup when invoked through the startup
entry point.

The socket peer is deliberately constructed to reproduce the stalled
handshake condition. These tests do not depend on reproducing an
intermittent CI failure.

CI status

For commit bbf02c17a90b2a23563bde8d02be9574201343a0, GitHub reports
continuous-integration/jenkins/pr-head as successful:

Jenkins PR-992, build 6

This records the reported CI status for that revision. It does not
establish that all historical startup failures share one cause or that
every platform-specific process-termination behavior has been tested.

Relationship to #990

This PR does not depend on the breakpoint-removal implementation in
#990 and
contains no breakpoint-specific changes.

The intended integration order is to review and merge this
infrastructure change first, then update #990 to the resulting upstream
state. This keeps the shared test-harness behavior changes separately
reviewable and out of the final breakpoint-specific diff.

Author checklist

• I have thoroughly tested my changes
• The change is following the coding conventions
• I have signed the Eclipse Contributor Agreement (ECA)

[Copilot]

🟡 Changes recommended

A small robustness bug in the new process-termination helper can throw UnsupportedOperationException from Process.pid() and mask the intended cleanup failure signal.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the JDI test harness against failed VM startup/attach/program-start scenarios by ensuring owned processes and reader threads are cleaned up even when setUp() aborts, and by bounding attach/termination waits to avoid hangs.

Changes:

• Add failure-safe startup cleanup in AbstractJDITest (including interrupt preservation and diagnostic retention on connect timeouts).
• Introduce bounded process termination helper and new regression tests covering injected failures and a real Java subprocess.
• Register the new regression tests in the JDI AutomatedSuite.

File summaries

File	Description
org.eclipse.jdt.debug.jdi.tests/tests/org/eclipse/debug/jdi/tests/VMStartupCleanupTest.java	Adds regression coverage for startup failure cleanup paths (processes, readers, ports, idempotent shutdown).
org.eclipse.jdt.debug.jdi.tests/tests/org/eclipse/debug/jdi/tests/VMConnectionTimeoutTest.java	Adds regression coverage ensuring a stalled JDWP handshake can’t hang startup and cleanup preserves diagnostics.
org.eclipse.jdt.debug.jdi.tests/tests/org/eclipse/debug/jdi/tests/TestProcessCleanup.java	Adds bounded termination utility used by the test harness to reliably stop owned processes.
org.eclipse.jdt.debug.jdi.tests/tests/org/eclipse/debug/jdi/tests/AutomatedSuite.java	Includes the new regression test classes in the automated suite.
org.eclipse.jdt.debug.jdi.tests/tests/org/eclipse/debug/jdi/tests/AbstractJDITest.java	Implements failure-safe cleanup on startup errors, bounded attach timeouts, and safer shutdown when readers/event reader may be absent.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.