From ba96f6c67569b3d35fa04cf8c038cb7c48333dda Mon Sep 17 00:00:00 2001 From: xiphon Date: Thu, 15 Nov 2018 15:16:19 +0000 Subject: [PATCH 1/7] debug: fix updating breakpoints while debuggee is running --- src/debugAdapter/goDebug.ts | 41 ++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index 042d5a3f1..f6a7c3ff1 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -534,6 +534,7 @@ class GoDebugSession extends LoggingDebugSession { private _variableHandles: Handles; private breakpoints: Map; + private skipStopEventOnce: boolean; private threads: Set; private debugState: DebuggerState; private delve: Delve; @@ -548,6 +549,7 @@ class GoDebugSession extends LoggingDebugSession { public constructor(debuggerLinesStartAt1: boolean, isServer: boolean = false) { super('', debuggerLinesStartAt1, isServer); this._variableHandles = new Handles(); + this.skipStopEventOnce = false; this.threads = new Set(); this.debugState = null; this.delve = null; @@ -694,15 +696,14 @@ class GoDebugSession extends LoggingDebugSession { return pathToConvert.replace(this.delve.remotePath, this.delve.program).split(this.remotePathSeparator).join(this.localPathSeparator); } - protected setBreakPointsRequest(response: DebugProtocol.SetBreakpointsResponse, args: DebugProtocol.SetBreakpointsArguments): void { - log('SetBreakPointsRequest'); + private setBreakPoints(response: DebugProtocol.SetBreakpointsResponse, args: DebugProtocol.SetBreakpointsArguments): Thenable { let file = normalizePath(args.source.path); if (!this.breakpoints.get(file)) { this.breakpoints.set(file, []); } let remoteFile = this.toDebuggerPath(file); - Promise.all(this.breakpoints.get(file).map(existingBP => { + return Promise.all(this.breakpoints.get(file).map(existingBP => { log('Clearing: ' + existingBP.id); return this.delve.callPromise('ClearBreakpoint', [this.delve.isApiV1 ? existingBP.id : { Id: existingBP.id }]); })).then(() => { @@ -751,6 +752,26 @@ class GoDebugSession extends LoggingDebugSession { }); } + protected setBreakPointsRequest(response: DebugProtocol.SetBreakpointsResponse, args: DebugProtocol.SetBreakpointsArguments): void { + log('SetBreakPointsRequest'); + if (!this.continueRequestRunning) { + this.setBreakPoints(response, args); + } else { + this.skipStopEventOnce = true; + this.delve.callPromise('Command', [{ name: 'halt' }]).then(() => { + return this.setBreakPoints(response, args).then(() => { + return this.continue(); + }, err => { + logError(err); + return this.sendErrorResponse(response, 2009, 'Failed to continue delve after setting the breakpoint: "{e}"', { e: err.toString() }); + }); + }, err => { + logError(err); + return this.sendErrorResponse(response, 2008, 'Failed to halt delve before attempting to set breakpoint: "{e}"', { e: err.toString() }); + }); + } + } + protected threadsRequest(response: DebugProtocol.ThreadsResponse): void { if (this.continueRequestRunning) { // Thread request to delve is syncronous and will block if a previous async continue request didn't return @@ -1073,6 +1094,11 @@ class GoDebugSession extends LoggingDebugSession { this.threads.delete(id); }); + if (this.skipStopEventOnce) { + this.skipStopEventOnce = false; + return; + } + let stoppedEvent = new StoppedEvent(reason, this.debugState.currentGoroutine.id); (stoppedEvent.body).allThreadsStopped = true; this.sendEvent(stoppedEvent); @@ -1080,10 +1106,10 @@ class GoDebugSession extends LoggingDebugSession { }); } } + private continueEpoch = 0; private continueRequestRunning = false; - protected continueRequest(response: DebugProtocol.ContinueResponse): void { - log('ContinueRequest'); + private continue(): void { this.continueEpoch++; let closureEpoch = this.continueEpoch; this.continueRequestRunning = true; @@ -1099,6 +1125,11 @@ class GoDebugSession extends LoggingDebugSession { this.debugState = state; this.handleReenterDebug('breakpoint'); }); + } + + protected continueRequest(response: DebugProtocol.ContinueResponse): void { + log('ContinueRequest'); + this.continue(); this.sendResponse(response); log('ContinueResponse'); } From 15a3332ab5d66e6d35523e2c028105f900fe7760 Mon Sep 17 00:00:00 2001 From: Ramya Achutha Rao Date: Thu, 17 Jan 2019 22:21:45 -0800 Subject: [PATCH 2/7] handle error when continuing --- src/debugAdapter/goDebug.ts | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index f6a7c3ff1..5b71ca04f 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -534,7 +534,7 @@ class GoDebugSession extends LoggingDebugSession { private _variableHandles: Handles; private breakpoints: Map; - private skipStopEventOnce: boolean; + private skipStopEventOnce: boolean; // Editing breakpoints requires halting delve, skip sending Stop Event to VS Code in such cases private threads: Set; private debugState: DebuggerState; private delve: Delve; @@ -760,10 +760,10 @@ class GoDebugSession extends LoggingDebugSession { this.skipStopEventOnce = true; this.delve.callPromise('Command', [{ name: 'halt' }]).then(() => { return this.setBreakPoints(response, args).then(() => { - return this.continue(); - }, err => { - logError(err); - return this.sendErrorResponse(response, 2009, 'Failed to continue delve after setting the breakpoint: "{e}"', { e: err.toString() }); + return this.continue(true).then(null, err => { + logError(err); + this.sendErrorResponse(response, 2009, 'Failed to continue delve after halting it to set breakpoints: "{e}"', { e: err.toString() }); + }); }); }, err => { logError(err); @@ -1109,27 +1109,39 @@ class GoDebugSession extends LoggingDebugSession { private continueEpoch = 0; private continueRequestRunning = false; - private continue(): void { + private continue(calledWhenSettingBreakpoint?: boolean): Thenable { this.continueEpoch++; let closureEpoch = this.continueEpoch; this.continueRequestRunning = true; - this.delve.call('Command', [{ name: 'continue' }], (err, out) => { + + const callback = (out) => { if (closureEpoch === this.continueEpoch) { this.continueRequestRunning = false; } - if (err) { - logError('Failed to continue - ' + err.toString()); - } const state = this.delve.isApiV1 ? out : (out).State; log('continue state', state); this.debugState = state; - this.handleReenterDebug('breakpoint'); + }; + + // If called when setting breakpoint internally, we want the error to bubble up. + if (calledWhenSettingBreakpoint) { + return this.delve.callPromise('Command', [{ name: 'continue' }]).then(callback); + } + return this.delve.callPromise('Command', [{ name: 'continue' }]).then(callback, err => { + if (closureEpoch === this.continueEpoch) { + this.continueRequestRunning = false; + } + if (err) { + logError('Failed to continue - ' + err.toString()); + } }); } protected continueRequest(response: DebugProtocol.ContinueResponse): void { log('ContinueRequest'); - this.continue(); + this.continue().then(() => { + this.handleReenterDebug('breakpoint'); + }); this.sendResponse(response); log('ContinueResponse'); } From 012b5358f7f525259292d67bb25b5e7b0e6cf121 Mon Sep 17 00:00:00 2001 From: Ramya Achutha Rao Date: Thu, 17 Jan 2019 22:27:57 -0800 Subject: [PATCH 3/7] Revert "handle error when continuing" This reverts commit 15a3332ab5d66e6d35523e2c028105f900fe7760. --- src/debugAdapter/goDebug.ts | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index 5b71ca04f..f6a7c3ff1 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -534,7 +534,7 @@ class GoDebugSession extends LoggingDebugSession { private _variableHandles: Handles; private breakpoints: Map; - private skipStopEventOnce: boolean; // Editing breakpoints requires halting delve, skip sending Stop Event to VS Code in such cases + private skipStopEventOnce: boolean; private threads: Set; private debugState: DebuggerState; private delve: Delve; @@ -760,10 +760,10 @@ class GoDebugSession extends LoggingDebugSession { this.skipStopEventOnce = true; this.delve.callPromise('Command', [{ name: 'halt' }]).then(() => { return this.setBreakPoints(response, args).then(() => { - return this.continue(true).then(null, err => { - logError(err); - this.sendErrorResponse(response, 2009, 'Failed to continue delve after halting it to set breakpoints: "{e}"', { e: err.toString() }); - }); + return this.continue(); + }, err => { + logError(err); + return this.sendErrorResponse(response, 2009, 'Failed to continue delve after setting the breakpoint: "{e}"', { e: err.toString() }); }); }, err => { logError(err); @@ -1109,39 +1109,27 @@ class GoDebugSession extends LoggingDebugSession { private continueEpoch = 0; private continueRequestRunning = false; - private continue(calledWhenSettingBreakpoint?: boolean): Thenable { + private continue(): void { this.continueEpoch++; let closureEpoch = this.continueEpoch; this.continueRequestRunning = true; - - const callback = (out) => { - if (closureEpoch === this.continueEpoch) { - this.continueRequestRunning = false; - } - const state = this.delve.isApiV1 ? out : (out).State; - log('continue state', state); - this.debugState = state; - }; - - // If called when setting breakpoint internally, we want the error to bubble up. - if (calledWhenSettingBreakpoint) { - return this.delve.callPromise('Command', [{ name: 'continue' }]).then(callback); - } - return this.delve.callPromise('Command', [{ name: 'continue' }]).then(callback, err => { + this.delve.call('Command', [{ name: 'continue' }], (err, out) => { if (closureEpoch === this.continueEpoch) { this.continueRequestRunning = false; } if (err) { logError('Failed to continue - ' + err.toString()); } + const state = this.delve.isApiV1 ? out : (out).State; + log('continue state', state); + this.debugState = state; + this.handleReenterDebug('breakpoint'); }); } protected continueRequest(response: DebugProtocol.ContinueResponse): void { log('ContinueRequest'); - this.continue().then(() => { - this.handleReenterDebug('breakpoint'); - }); + this.continue(); this.sendResponse(response); log('ContinueResponse'); } From 45e6b64893f9b3c4b0b090adde27ce2955b9facb Mon Sep 17 00:00:00 2001 From: Ramya Achutha Rao Date: Thu, 17 Jan 2019 22:39:49 -0800 Subject: [PATCH 4/7] Handle error from continue() --- src/debugAdapter/goDebug.ts | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index f6a7c3ff1..05b9a7fae 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -534,7 +534,7 @@ class GoDebugSession extends LoggingDebugSession { private _variableHandles: Handles; private breakpoints: Map; - private skipStopEventOnce: boolean; + private skipStopEventOnce: boolean; // Editing breakpoints requires halting delve, skip sending Stop Event to VS Code in such cases private threads: Set; private debugState: DebuggerState; private delve: Delve; @@ -760,10 +760,10 @@ class GoDebugSession extends LoggingDebugSession { this.skipStopEventOnce = true; this.delve.callPromise('Command', [{ name: 'halt' }]).then(() => { return this.setBreakPoints(response, args).then(() => { - return this.continue(); - }, err => { - logError(err); - return this.sendErrorResponse(response, 2009, 'Failed to continue delve after setting the breakpoint: "{e}"', { e: err.toString() }); + return this.continue(true).then(null, err => { + logError(err); + this.sendErrorResponse(response, 2009, 'Failed to continue delve after halting it to set breakpoints: "{e}"', { e: err.toString() }); + }); }); }, err => { logError(err); @@ -1109,22 +1109,25 @@ class GoDebugSession extends LoggingDebugSession { private continueEpoch = 0; private continueRequestRunning = false; - private continue(): void { + private continue(calledWhenSettingBreakpoint?: boolean): Thenable { this.continueEpoch++; let closureEpoch = this.continueEpoch; this.continueRequestRunning = true; - this.delve.call('Command', [{ name: 'continue' }], (err, out) => { + + const callback = (out) => { if (closureEpoch === this.continueEpoch) { this.continueRequestRunning = false; } - if (err) { - logError('Failed to continue - ' + err.toString()); - } const state = this.delve.isApiV1 ? out : (out).State; log('continue state', state); this.debugState = state; this.handleReenterDebug('breakpoint'); - }); + }; + + // If called when setting breakpoint internally, we want the error to bubble up. + const errorCallback = calledWhenSettingBreakpoint ? null : callback; + + return this.delve.callPromise('Command', [{ name: 'continue' }]).then(callback, errorCallback); } protected continueRequest(response: DebugProtocol.ContinueResponse): void { From 4226264c13b8ad79359bb1e9e361757c169cde22 Mon Sep 17 00:00:00 2001 From: Ramya Achutha Rao Date: Thu, 17 Jan 2019 22:44:35 -0800 Subject: [PATCH 5/7] Log error when continue fails in the normal case --- src/debugAdapter/goDebug.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index 05b9a7fae..d139df5a2 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -1125,7 +1125,12 @@ class GoDebugSession extends LoggingDebugSession { }; // If called when setting breakpoint internally, we want the error to bubble up. - const errorCallback = calledWhenSettingBreakpoint ? null : callback; + const errorCallback = calledWhenSettingBreakpoint ? null : (err) => { + if (err) { + logError('Failed to continue - ' + err.toString()); + } + this.handleReenterDebug('breakpoint'); + }; return this.delve.callPromise('Command', [{ name: 'continue' }]).then(callback, errorCallback); } From 009739f2f38d6394f3a337d75baabb9d36f5810c Mon Sep 17 00:00:00 2001 From: xiphon Date: Sat, 19 Jan 2019 00:06:41 +0000 Subject: [PATCH 6/7] debug: revert 'skipStopEventOnce' on delve 'halt' error --- src/debugAdapter/goDebug.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index d139df5a2..933642753 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -766,6 +766,7 @@ class GoDebugSession extends LoggingDebugSession { }); }); }, err => { + this.skipStopEventOnce = false; logError(err); return this.sendErrorResponse(response, 2008, 'Failed to halt delve before attempting to set breakpoint: "{e}"', { e: err.toString() }); }); From 3a53f34c8fb40dccdfe576dd67419724f5328b83 Mon Sep 17 00:00:00 2001 From: xiphon Date: Sat, 19 Jan 2019 00:07:23 +0000 Subject: [PATCH 7/7] debug: don't send error reponse twice on 'continue' error --- src/debugAdapter/goDebug.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index 933642753..b81df2fcc 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -761,8 +761,7 @@ class GoDebugSession extends LoggingDebugSession { this.delve.callPromise('Command', [{ name: 'halt' }]).then(() => { return this.setBreakPoints(response, args).then(() => { return this.continue(true).then(null, err => { - logError(err); - this.sendErrorResponse(response, 2009, 'Failed to continue delve after halting it to set breakpoints: "{e}"', { e: err.toString() }); + logError(`Failed to continue delve after halting it to set breakpoints: "${err.toString()}"`); }); }); }, err => { @@ -1131,6 +1130,7 @@ class GoDebugSession extends LoggingDebugSession { logError('Failed to continue - ' + err.toString()); } this.handleReenterDebug('breakpoint'); + throw err; }; return this.delve.callPromise('Command', [{ name: 'continue' }]).then(callback, errorCallback);