src/debugAdapter: report that next is automatically cancelled if interrupted
In order to set breakpoints, a running program must be halted. When a next,
step in, or step out request is interrupted by a halt, the request
is automatically cancelled. Unlike the case when continue is interrupted,
we should not suppress a stop event and automatically continue.
Instead we issue a stopped event and log a warning to stderr.
Test that the debug adapter is able to set breakpoint requests
while the program is running continue, next, and step out requests.
Updates golang/vscode-go#787
Change-Id: I1b17a65d15fd35c628b1fa2d823c558a24a84727
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/261078
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Polina Sokolova <polina@google.com>
diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts
index ac3ece0..72d1772 100644
--- a/src/debugAdapter/goDebug.ts
+++ b/src/debugAdapter/goDebug.ts
@@ -90,6 +90,7 @@
currentGoroutine: DebugGoroutine;
Running: boolean;
Threads: DebugThread[];
+ NextInProgress: boolean;
}
export interface PackageBuildInfo {
@@ -857,6 +858,7 @@
private breakpoints: Map<string, DebugBreakpoint[]>;
// Editing breakpoints requires halting delve, skip sending Stop Event to VS Code in such cases
private skipStopEventOnce: boolean;
+ private overrideStopReason: string;
private debugState: DebuggerState;
private delve: Delve;
private localPathSeparator: string;
@@ -877,6 +879,8 @@
private continueEpoch = 0;
private continueRequestRunning = false;
+ private nextEpoch = 0;
+ private nextRequestRunning = false;
public constructor(
debuggerLinesStartAt1: boolean,
isServer: boolean = false,
@@ -884,6 +888,7 @@
super('', debuggerLinesStartAt1, isServer);
this.variableHandles = new Handles<DebugVariable>();
this.skipStopEventOnce = false;
+ this.overrideStopReason = '';
this.stopOnEntry = false;
this.debugState = null;
this.delve = null;
@@ -1303,11 +1308,25 @@
log('Debuggee is not running. Setting breakpoints without halting.');
await this.setBreakPoints(response, args);
} else {
+ // Skip stop event if a continue request is running.
this.skipStopEventOnce = this.continueRequestRunning;
+ const haltedDuringNext = this.nextRequestRunning;
+ if (haltedDuringNext) {
+ this.overrideStopReason = 'next cancelled';
+ }
+
log(`Halting before setting breakpoints. SkipStopEventOnce is ${this.skipStopEventOnce}.`);
this.delve.callPromise('Command', [{ name: 'halt' }]).then(
() => {
return this.setBreakPoints(response, args).then(() => {
+ // We do not want to continue if it was running a next request, since the
+ // request was automatically cancelled.
+ if (haltedDuringNext) {
+ // Send an output event containing a warning that next was cancelled.
+ const warning = `Setting breakpoints during 'next', 'step in' or 'step out' halted delve and cancelled the next request`;
+ this.sendEvent(new OutputEvent(warning, 'stderr'));
+ return;
+ }
return this.continue(true).then(null, (err) => {
this.logDelveError(err, 'Failed to continue delve after halting it to set breakpoints');
});
@@ -1687,8 +1706,16 @@
}
protected nextRequest(response: DebugProtocol.NextResponse): void {
+ this.nextEpoch++;
+ const closureEpoch = this.nextEpoch;
+ this.nextRequestRunning = true;
+
log('NextRequest');
this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'next' }], (err, out) => {
+ if (closureEpoch === this.continueEpoch) {
+ this.nextRequestRunning = false;
+ }
+
if (err) {
this.logDelveError(err, 'Failed to next');
}
@@ -1702,8 +1729,16 @@
}
protected stepInRequest(response: DebugProtocol.StepInResponse): void {
+ this.nextEpoch++;
+ const closureEpoch = this.nextEpoch;
+ this.nextRequestRunning = true;
+
log('StepInRequest');
this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'step' }], (err, out) => {
+ if (closureEpoch === this.continueEpoch) {
+ this.nextRequestRunning = false;
+ }
+
if (err) {
this.logDelveError(err, 'Failed to step in');
}
@@ -1717,8 +1752,16 @@
}
protected stepOutRequest(response: DebugProtocol.StepOutResponse): void {
+ this.nextEpoch++;
+ const closureEpoch = this.nextEpoch;
+ this.nextRequestRunning = true;
+
log('StepOutRequest');
this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'stepOut' }], (err, out) => {
+ if (closureEpoch === this.continueEpoch) {
+ this.nextRequestRunning = false;
+ }
+
if (err) {
this.logDelveError(err, 'Failed to step out');
}
@@ -2352,6 +2395,11 @@
return;
}
+ if (this.overrideStopReason?.length > 0) {
+ reason = this.overrideStopReason;
+ this.overrideStopReason = '';
+ }
+
const stoppedEvent = new StoppedEvent(reason, this.debugState.currentGoroutine.id);
(<any>stoppedEvent.body).allThreadsStopped = true;
this.sendEvent(stoppedEvent);
@@ -2375,7 +2423,7 @@
} catch (error) {
this.logDelveError(error, 'Failed to get state');
// Fall back to the internal tracking.
- return this.continueRequestRunning;
+ return this.continueRequestRunning || this.nextRequestRunning;
}
}
diff --git a/test/integration/goDebug.test.ts b/test/integration/goDebug.test.ts
index 15142df..fbf775b 100644
--- a/test/integration/goDebug.test.ts
+++ b/test/integration/goDebug.test.ts
@@ -880,6 +880,117 @@
await killProcessTree(remoteProgram);
await new Promise((resolve) => setTimeout(resolve, 2_000));
});
+
+ test('stopped for a breakpoint set during initialization (remote attach)', async () => {
+ const FILE = path.join(DATA_ROOT, 'helloWorldServer', 'main.go');
+ const BREAKPOINT_LINE = 29;
+ const remoteProgram = await setUpRemoteProgram(remoteAttachConfig.port, server);
+
+ const breakpointLocation = getBreakpointLocation(FILE, BREAKPOINT_LINE, false);
+
+ // Setup attach with a breakpoint.
+ await setUpRemoteAttach(remoteAttachDebugConfig, [breakpointLocation]);
+
+ // Calls the helloworld server to make the breakpoint hit.
+ await waitForBreakpoint(
+ () => http.get(`http://localhost:${server}`).on('error', (data) => console.log(data)),
+ breakpointLocation);
+
+ await dc.disconnectRequest({restart: false});
+ await killProcessTree(remoteProgram);
+ await new Promise((resolve) => setTimeout(resolve, 2_000));
+ });
+
+ test('should set breakpoints during continue', async () => {
+ const PROGRAM = path.join(DATA_ROOT, 'sleep');
+
+ const FILE = path.join(DATA_ROOT, 'sleep', 'sleep.go');
+ const HELLO_LINE = 10;
+ const helloLocation = getBreakpointLocation(FILE, HELLO_LINE);
+
+ const config = {
+ name: 'Launch file',
+ type: 'go',
+ request: 'launch',
+ mode: 'auto',
+ program: PROGRAM
+ };
+ const debugConfig = debugConfigProvider.resolveDebugConfiguration(undefined, config);
+
+ await Promise.all([
+ dc.configurationSequence(),
+ dc.launch(debugConfig),
+ ]);
+
+ return Promise.all([
+ dc.setBreakpointsRequest({
+ lines: [ helloLocation.line ],
+ breakpoints: [ { line: helloLocation.line, column: 0 } ],
+ source: { path: helloLocation.path }
+ }),
+ dc.assertStoppedLocation('breakpoint', helloLocation)
+ ]);
+ });
+
+ async function setBreakpointsDuringStep(nextFunc: () => void) {
+ const PROGRAM = path.join(DATA_ROOT, 'sleep');
+
+ const FILE = path.join(DATA_ROOT, 'sleep', 'sleep.go');
+ const SLEEP_LINE = 11;
+ const setupBreakpoint = getBreakpointLocation(FILE, SLEEP_LINE);
+
+ const HELLO_LINE = 10;
+ const onNextBreakpoint = getBreakpointLocation(FILE, HELLO_LINE);
+
+ const config = {
+ name: 'Launch file',
+ type: 'go',
+ request: 'launch',
+ mode: 'auto',
+ program: PROGRAM
+ };
+ const debugConfig = debugConfigProvider.resolveDebugConfiguration(undefined, config);
+
+ await dc.hitBreakpoint(debugConfig, setupBreakpoint);
+
+ // The program is now stopped at the line containing time.Sleep().
+ // Issue a next request, followed by a setBreakpointsRequest.
+ nextFunc();
+
+ // Note: the current behavior of setting a breakpoint during a next
+ // request will cause the step to be interrupted, so it may not be
+ // stopped on the next line.
+ await Promise.all([
+ dc.setBreakpointsRequest({
+ lines: [ onNextBreakpoint.line ],
+ breakpoints: [ { line: onNextBreakpoint.line, column: 0 } ],
+ source: { path: onNextBreakpoint.path }
+ }),
+ dc.assertStoppedLocation('next cancelled', {})
+ ]);
+
+ // Once the 'step' has completed, continue the program and
+ // make sure the breakpoint set while the program was nexting
+ // is succesfully hit.
+ await Promise.all([
+ dc.continueRequest({threadId: 1}),
+ dc.assertStoppedLocation('breakpoint', onNextBreakpoint)
+ ]);
+ }
+
+ test('should set breakpoints during next', async () => {
+ setBreakpointsDuringStep(async () => {
+ const nextResponse = await dc.nextRequest({threadId: 1});
+ assert.ok(nextResponse.success);
+ });
+ });
+
+ test('should set breakpoints during step out', async () => {
+ setBreakpointsDuringStep(async () => {
+ const stepOutResponse = await dc.stepOutRequest({threadId: 1});
+ assert.ok(stepOutResponse.success);
+ });
+ });
});
suite('conditionalBreakpoints', () => {