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 #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>

Author: suzmue

src/debugAdapter/goDebug.ts

@@ -90,6 +90,7 @@ interface DebuggerState {
 	currentGoroutine: DebugGoroutine;
 	Running: boolean;
 	Threads: DebugThread[];
+	NextInProgress: boolean;
 }
 
 export interface PackageBuildInfo {
@@ -857,6 +858,7 @@ export class GoDebugSession extends LoggingDebugSession {
 	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,13 +879,16 @@ export class GoDebugSession extends LoggingDebugSession {
 
 	private continueEpoch = 0;
 	private continueRequestRunning = false;
+	private nextEpoch = 0;
+	private nextRequestRunning = false;
 	public constructor(
 		debuggerLinesStartAt1: boolean,
 		isServer: boolean = false,
 		readonly fileSystem = fs) {
 		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 @@ export class GoDebugSession extends LoggingDebugSession {
 			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 @@ export class GoDebugSession extends LoggingDebugSession {
 	}
 
 	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 @@ export class GoDebugSession extends LoggingDebugSession {
 	}
 
 	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 @@ export class GoDebugSession extends LoggingDebugSession {
 	}
 
 	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 @@ export class GoDebugSession extends LoggingDebugSession {
 					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 @@ export class GoDebugSession extends LoggingDebugSession {
 		} catch (error) {
 			this.logDelveError(error, 'Failed to get state');
 			// Fall back to the internal tracking.
-			return this.continueRequestRunning;
+			return this.continueRequestRunning || this.nextRequestRunning;
 		}
 	}
 

test/integration/goDebug.test.ts

@@ -880,6 +880,117 @@ suite('Go Debug Adapter', function () {
 			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', () => {