diff --git a/src/desktop/GDBTargetDebugSession.ts b/src/desktop/GDBTargetDebugSession.ts index 169ffb19..f532c83a 100644 --- a/src/desktop/GDBTargetDebugSession.ts +++ b/src/desktop/GDBTargetDebugSession.ts @@ -462,6 +462,10 @@ export class GDBTargetDebugSession extends GDBDebugSession { }); } + /** + * WARNING: `disconnectRequest` is unreliable in sync mode. + * @see {@link https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/pull/339#discussion_r1840549671} + */ protected async disconnectRequest( response: DebugProtocol.DisconnectResponse, _args: DebugProtocol.DisconnectArguments @@ -471,15 +475,8 @@ export class GDBTargetDebugSession extends GDBDebugSession { this.serialPort.close(); if (this.targetType === 'remote') { - if (this.gdb.getAsyncMode() && this.isRunning) { - // See #295 - this use of "await" is to try to slightly delay the - // call to disconnect. A proper solution that waits for the - // interrupt to be successful is needed to avoid future - // "Cannot execute this command while the target is running" - // errors - await this.gdb.sendCommand('interrupt'); - } - + // Need to pause first, then disconnect and exit + await this.pauseIfNeeded(true); await this.gdb.sendCommand('disconnect'); } diff --git a/src/gdb/GDBDebugSessionBase.ts b/src/gdb/GDBDebugSessionBase.ts index f3283777..97cfa45a 100644 --- a/src/gdb/GDBDebugSessionBase.ts +++ b/src/gdb/GDBDebugSessionBase.ts @@ -328,13 +328,29 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { return this.gdb?.spawn(args); } - protected async setBreakPointsRequest( - response: DebugProtocol.SetBreakpointsResponse, - args: DebugProtocol.SetBreakpointsArguments - ): Promise { - this.waitPausedNeeded = this.isRunning; + /** + * Sends a pause command to GDBBackend, and resolves when the debugger is + * actually paused. The paused thread ID is saved to `this.waitPausedThreadId`. + * @param requireAsync - require gdb to be in async mode to pause + */ + protected async pauseIfNeeded(requireAsync?: false): Promise; + + /** + * Sends a pause command to GDBBackend, and resolves when the debugger is + * actually paused. The paused thread ID is saved to `this.waitPausedThreadId`. + * + * @param requireAsync - require gdb to be in async mode to pause + * @deprecated the `requireAsync` parameter should not be used and will be + * removed in the future. + * See {@link https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/pull/339#discussion_r1840549671} + */ + protected async pauseIfNeeded(requireAsync: true): Promise; + + protected async pauseIfNeeded(requireAsync = false): Promise { + this.waitPausedNeeded = + this.isRunning && (!requireAsync || this.gdb.getAsyncMode()); + if (this.waitPausedNeeded) { - // Need to pause first const waitPromise = new Promise((resolve) => { this.waitPaused = resolve; }); @@ -349,8 +365,29 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { } else { this.gdb.pause(); } + + // This promise resolves when handling GDBAsync for the "stopped" + // result class, which indicates that the call to `GDBBackend::pause` + // is actually finished. await waitPromise; } + } + + protected async continueIfNeeded(): Promise { + if (this.waitPausedNeeded) { + if (this.gdb.isNonStopMode()) { + await mi.sendExecContinue(this.gdb, this.waitPausedThreadId); + } else { + await mi.sendExecContinue(this.gdb); + } + } + } + + protected async setBreakPointsRequest( + response: DebugProtocol.SetBreakpointsResponse, + args: DebugProtocol.SetBreakpointsArguments + ): Promise { + await this.pauseIfNeeded(); try { // Need to get the list of current breakpoints in the file and then make sure @@ -518,38 +555,14 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { ); } - if (this.waitPausedNeeded) { - if (this.gdb.isNonStopMode()) { - mi.sendExecContinue(this.gdb, this.waitPausedThreadId); - } else { - mi.sendExecContinue(this.gdb); - } - } + await this.continueIfNeeded(); } protected async setFunctionBreakPointsRequest( response: DebugProtocol.SetFunctionBreakpointsResponse, args: DebugProtocol.SetFunctionBreakpointsArguments ) { - this.waitPausedNeeded = this.isRunning; - if (this.waitPausedNeeded) { - // Need to pause first - const waitPromise = new Promise((resolve) => { - this.waitPaused = resolve; - }); - if (this.gdb.isNonStopMode()) { - const threadInfo = await mi.sendThreadInfoRequest(this.gdb, {}); - - this.waitPausedThreadId = parseInt( - threadInfo['current-thread-id'], - 10 - ); - this.gdb.pause(this.waitPausedThreadId); - } else { - this.gdb.pause(); - } - await waitPromise; - } + await this.pauseIfNeeded(); try { const result = await mi.sendBreakList(this.gdb); @@ -645,13 +658,7 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession { ); } - if (this.waitPausedNeeded) { - if (this.gdb.isNonStopMode()) { - mi.sendExecContinue(this.gdb, this.waitPausedThreadId); - } else { - mi.sendExecContinue(this.gdb); - } - } + await this.continueIfNeeded(); } /** diff --git a/src/web/GDBTargetDebugSession.ts b/src/web/GDBTargetDebugSession.ts index 4049b395..3f9ef181 100644 --- a/src/web/GDBTargetDebugSession.ts +++ b/src/web/GDBTargetDebugSession.ts @@ -320,21 +320,18 @@ export class GDBTargetDebugSession extends GDBDebugSession { return this.gdbserverProcessManager?.stop(); } + /** + * WARNING: `disconnectRequest` is unreliable in sync mode. + * @see {@link https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/pull/339#discussion_r1840549671} + */ protected async disconnectRequest( response: DebugProtocol.DisconnectResponse, _args: DebugProtocol.DisconnectArguments ): Promise { try { if (this.targetType === 'remote') { - if (this.gdb.getAsyncMode() && this.isRunning) { - // See #295 - this use of "await" is to try to slightly delay the - // call to disconnect. A proper solution that waits for the - // interrupt to be successful is needed to avoid future - // "Cannot execute this command while the target is running" - // errors - await this.gdb.sendCommand('interrupt'); - } - + // Need to pause first, then disconnect and exit + await this.pauseIfNeeded(true); await this.gdb.sendCommand('disconnect'); }