Skip to content

Commit 354ad3a

Browse files
sebthomsebthom
committed
fix(debug): defer adapter shutdown until exited event
Stop handling DAP terminated by tearing down the debug adapter connection. Perform final cleanup in DSPDebugTarget.exited and mark DSPProcess terminated without sending terminate/disconnect.
1 parent fb5de3e commit 354ad3a

3 files changed

Lines changed: 225 additions & 7 deletions

File tree

‎org.eclipse.lsp4e.debug/src/org/eclipse/lsp4e/debug/console/DSPProcess.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ public boolean isTerminated() {
6565
return handle.map(h -> !h.isAlive()).orElse(terminated);
6666
}
6767

68+
public void markTerminatedByTarget() {
69+
terminated = true;
70+
DebugPlugin.getDefault().fireDebugEventSet(new DebugEvent[] { new DebugEvent(this, DebugEvent.TERMINATE) });
71+
}
72+
6873
@Override
6974
public void terminate() throws DebugException {
7075
terminated = true;

‎org.eclipse.lsp4e.debug/src/org/eclipse/lsp4e/debug/debugmodel/DSPDebugTarget.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -321,11 +321,7 @@ private void terminated() {
321321
}
322322
final var process = this.process;
323323
if (process != null && process.canTerminate()) {
324-
try {
325-
process.terminate();
326-
} catch (DebugException e) {
327-
DSPPlugin.logError(e);
328-
}
324+
process.markTerminatedByTarget();
329325
}
330326
fireTerminateEvent();
331327
debuggees.forEach(DSPDebugTarget::terminated);
@@ -406,7 +402,14 @@ public boolean isTerminated() {
406402

407403
@Override
408404
public void terminated(TerminatedEventArguments body) {
409-
terminated();
405+
/*
406+
* According to the Debug Adapter Protocol, the 'terminated' event signals that
407+
* the debuggee has finished, but the debug adapter may continue to run in
408+
* order to shut down cleanly and emit a corresponding 'exited' event. Do not
409+
* tear down the adapter connection here so that a subsequent 'exited' event
410+
* can still be delivered. Final cleanup is triggered from {@link #exited} or
411+
* from an explicit terminate/disconnect request initiated by the client.
412+
*/
410413
}
411414

412415
/**
@@ -591,7 +594,12 @@ public boolean supportsBreakpoint(IBreakpoint breakpoint) {
591594

592595
@Override
593596
public void exited(ExitedEventArguments args) {
594-
// TODO
597+
/*
598+
* The debug adapter reports that the debuggee has exited. At this point it is
599+
* safe to terminate the debug session and dispose the connection to the
600+
* adapter.
601+
*/
602+
terminated();
595603
}
596604

597605
@Override

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/debug/DebugSessionTerminationTest.java‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation.
11+
*******************************************************************************/
12+
package org.eclipse.lsp4e.test.debug;
13+
14+
import static org.junit.Assert.*;
15+
16+
import java.io.InputStream;
17+
import java.io.OutputStream;
18+
import java.util.HashMap;
19+
import java.util.Map;
20+
import java.util.concurrent.CompletableFuture;
21+
import java.util.concurrent.ExecutorService;
22+
import java.util.concurrent.atomic.AtomicBoolean;
23+
import java.util.concurrent.atomic.AtomicInteger;
24+
import java.util.function.UnaryOperator;
25+
26+
import org.eclipse.core.runtime.NullProgressMonitor;
27+
import org.eclipse.debug.core.DebugPlugin;
28+
import org.eclipse.debug.core.ILaunch;
29+
import org.eclipse.debug.core.ILaunchConfigurationType;
30+
import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
31+
import org.eclipse.debug.core.ILaunchManager;
32+
import org.eclipse.debug.core.Launch;
33+
import org.eclipse.lsp4e.debug.debugmodel.DSPDebugTarget;
34+
import org.eclipse.lsp4e.debug.debugmodel.TransportStreams;
35+
import org.eclipse.lsp4e.test.utils.AbstractTestWithProject;
36+
import org.eclipse.lsp4e.test.utils.TestUtils;
37+
import org.eclipse.lsp4j.debug.Capabilities;
38+
import org.eclipse.lsp4j.debug.DisconnectArguments;
39+
import org.eclipse.lsp4j.debug.ExitedEventArguments;
40+
import org.eclipse.lsp4j.debug.InitializeRequestArguments;
41+
import org.eclipse.lsp4j.debug.ProcessEventArguments;
42+
import org.eclipse.lsp4j.debug.TerminateArguments;
43+
import org.eclipse.lsp4j.debug.TerminatedEventArguments;
44+
import org.eclipse.lsp4j.debug.services.IDebugProtocolClient;
45+
import org.eclipse.lsp4j.debug.services.IDebugProtocolServer;
46+
import org.eclipse.lsp4j.jsonrpc.Launcher;
47+
import org.eclipse.lsp4j.jsonrpc.MessageConsumer;
48+
import org.eclipse.lsp4j.jsonrpc.RemoteEndpoint;
49+
import org.junit.Test;
50+
51+
/**
52+
* Regression test for https://github.com/eclipse-lsp4e/lsp4e/issues/266:
53+
* ensure LSP4E.debug does not terminate the debug adapter too early.
54+
*
55+
* Scenario:
56+
* <ol>
57+
* <li>create a mock DAP server and wire it into a DSPDebugTarget via a Launcher
58+
* stub (no real IO)
59+
* <li>server sends a ProcessEvent so LSP4E creates a DSPProcess
60+
* <li>server sends a terminated event followed by an exited event
61+
* <li>assert that the exited event can still be delivered without the
62+
* connection being torn down
63+
* </ol>
64+
*/
65+
public class DebugSessionTerminationTest extends AbstractTestWithProject {
66+
67+
/**
68+
* Minimal in-memory mock of a DAP server sufficient for this test.
69+
*/
70+
private static final class MockDebugServer implements IDebugProtocolServer {
71+
IDebugProtocolClient client;
72+
final AtomicBoolean exitedDelivered = new AtomicBoolean(false);
73+
final AtomicInteger terminateRequestCount = new AtomicInteger(0);
74+
final AtomicInteger disconnectRequestCount = new AtomicInteger(0);
75+
76+
@Override
77+
public CompletableFuture<Capabilities> initialize(InitializeRequestArguments args) {
78+
var caps = new Capabilities();
79+
// Advertise support for terminate so that DSPDebugTarget will send a
80+
// terminate request when it (incorrectly) decides to terminate the adapter.
81+
caps.setSupportsTerminateRequest(true);
82+
if (client != null) {
83+
client.initialized();
84+
}
85+
return CompletableFuture.completedFuture(caps);
86+
}
87+
88+
@Override
89+
public CompletableFuture<Void> launch(Map<String, Object> args) {
90+
if (client != null) {
91+
var process = new ProcessEventArguments();
92+
process.setName("mock-debuggee");
93+
client.process(process);
94+
95+
var terminated = new TerminatedEventArguments();
96+
client.terminated(terminated);
97+
98+
var exited = new ExitedEventArguments();
99+
exited.setExitCode(0);
100+
try {
101+
client.exited(exited);
102+
exitedDelivered.set(true);
103+
} catch (Throwable t) {
104+
exitedDelivered.set(false);
105+
}
106+
}
107+
return CompletableFuture.completedFuture(null);
108+
}
109+
110+
@Override
111+
public CompletableFuture<Void> terminate(TerminateArguments args) {
112+
terminateRequestCount.incrementAndGet();
113+
return CompletableFuture.completedFuture(null);
114+
}
115+
116+
@Override
117+
public CompletableFuture<Void> disconnect(DisconnectArguments args) {
118+
disconnectRequestCount.incrementAndGet();
119+
return CompletableFuture.completedFuture(null);
120+
}
121+
}
122+
123+
/**
124+
* DSPDebugTarget variant that injects a mock server without real JSON-RPC IO.
125+
*/
126+
private static final class TestDebugTarget extends DSPDebugTarget {
127+
private final IDebugProtocolServer server;
128+
129+
TestDebugTarget(ILaunch launch, Map<String, Object> dspParameters, IDebugProtocolServer server) {
130+
super(launch, () -> new TransportStreams.DefaultTransportStreams(InputStream.nullInputStream(),
131+
OutputStream.nullOutputStream()), dspParameters);
132+
this.server = server;
133+
}
134+
135+
@Override
136+
protected Launcher<? extends IDebugProtocolServer> createLauncher(UnaryOperator<MessageConsumer> wrapper,
137+
InputStream in, OutputStream out, ExecutorService threadPool) {
138+
if (server instanceof MockDebugServer m) {
139+
m.client = this;
140+
}
141+
return new Launcher<>() {
142+
@Override
143+
public RemoteEndpoint getRemoteEndpoint() {
144+
return null;
145+
}
146+
147+
@Override
148+
public IDebugProtocolServer getRemoteProxy() {
149+
return server;
150+
}
151+
152+
@Override
153+
public CompletableFuture<Void> startListening() {
154+
return CompletableFuture.completedFuture(null);
155+
}
156+
};
157+
}
158+
}
159+
160+
private static final String LAUNCH_TYPE_ID = "org.eclipse.lsp4e.debug.launchType";
161+
162+
private static ILaunch newLaunch(String mode) throws Exception {
163+
ILaunchConfigurationType type = DebugPlugin.getDefault().getLaunchManager()
164+
.getLaunchConfigurationType(LAUNCH_TYPE_ID);
165+
ILaunchConfigurationWorkingCopy wc = type.newInstance(null,
166+
"DebugSessionTerminationTest-" + System.currentTimeMillis());
167+
return new Launch(wc, mode, null);
168+
}
169+
170+
@Test
171+
public void testTerminatedDoesNotPreventExited() throws Exception {
172+
ILaunch launch = newLaunch(ILaunchManager.RUN_MODE);
173+
174+
var params = new HashMap<String, Object>();
175+
params.put("type", "mock");
176+
params.put("request", "launch");
177+
params.put("program", "dummy");
178+
179+
var server = new MockDebugServer();
180+
var target = new TestDebugTarget(launch, params, server);
181+
182+
target.initialize(new NullProgressMonitor());
183+
184+
// Wait until the terminated event from the adapter has been processed.
185+
TestUtils.waitForAndAssertCondition(5000, target::isTerminated);
186+
187+
// Sanity check: the target should be marked as terminated.
188+
assertTrue("Debug target should be terminated", target.isTerminated());
189+
190+
// The exited event should still be deliverable after terminated.
191+
assertTrue("Debug adapter exited event should be deliverable after terminated", server.exitedDelivered.get());
192+
193+
// Bug 266 history: LSP4E used to react to a 'terminated' event from the
194+
// adapter by calling DSPProcess.terminate(), which in turn called
195+
// DSPDebugTarget.terminate() and sent a terminate or disconnect request back
196+
// to the adapter. This was contrary to the DAP guidelines where a 'terminated'
197+
// event indicates that the debuggee has ended and the adapter should be
198+
// allowed to shut down cleanly (emitting 'exited'). This test ensures no such
199+
// terminate or disconnect request is sent in response to the adapter's event.
200+
assertEquals("LSP4E must not send terminate request back to adapter after receiving terminated event", 0,
201+
server.terminateRequestCount.get());
202+
assertEquals("LSP4E must not send disconnect request back to adapter after receiving terminated event", 0,
203+
server.disconnectRequestCount.get());
204+
}
205+
}

0 commit comments

Comments
 (0)