Silently pause target before reset if running (#464)

jreineckearm · web-flow · commit fced49e424a3 · 2025-11-13T15:27:58.000+01:00
* Adds pauseIfRunning method which does not expect
  continue after following debug operation.
* Call pauseIfRunning before execution of
  customResetCommands.
* Validate gdbAsync setting as required when using
   customResetCommands.

---------

Signed-off-by: Jens Reinecke &lt;jens.reinecke@arm.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Fixes [`463`](https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/issues/463): Cannot use custom reset while CPU is running.
 - Fixes [`465`](https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/issues/465): UTF-8 'Failed to decode cstring' errors for GDB with CP1252 support only.
 
 ## 1.4.1
diff --git a/src/desktop/GDBTargetDebugSession.ts b/src/desktop/GDBTargetDebugSession.ts
@@ -187,15 +187,20 @@ export class GDBTargetDebugSession extends GDBDebugSession {
             // Limitations for auxiliary GDB mode
             if (args.gdbNonStop) {
                 throw new Error(
-                    'Cannot use auxiliaryGdb mode with gdbNonStop mode'
+                    "Cannot use setting 'auxiliaryGdb' with 'gdbNonStop' mode"
                 );
             }
             if (args.gdbAsync === false) {
                 throw new Error(
-                    'AuxiliaryGdb mode requires gdbAsync to be active'
+                    "Setting 'auxiliaryGdb' mode requires 'gdbAsync' to be active"
                 );
             }
         }
+        if (args.customResetCommands?.length && args.gdbAsync === false) {
+            throw new Error(
+                "Setting 'customResetCommands' requires 'gdbAsync' to be active"
+            );
+        }
     }
 
     protected override async setupCommonLoggerAndBackends(
diff --git a/src/gdb/GDBDebugSessionBase.ts b/src/gdb/GDBDebugSessionBase.ts
@@ -85,6 +85,12 @@ interface StreamOutput {
     category: string;
 }
 
+// Interface for a pending pause request, used with pauseIfRunning() logic
+interface PendingPauseRequest {
+    threadId: number | undefined; // All threads if undefined
+    resolveFunc?: (value: void | PromiseLike<void>) => void;
+}
+
 export function hexToBase64(hex: string): string {
     // The buffer will ignore incomplete bytes (unpaired digits), so we need to catch that early
     if (hex.length % 2 !== 0) {
@@ -153,18 +159,27 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
     protected threads: ThreadWithStatus[] = [];
     protected missingThreadNames = false;
 
+    /**
+     * State variables for pauseIfNeeded/continueIfNeeded logic, mostly used for
+     * temporary pause to insert breakpoints while the target is running.
+     */
     // promise that resolves once the target stops so breakpoints can be inserted
     protected waitPausedPromise?: Promise<void>;
     // resolve function of waitPausedPromise while waiting, undefined otherwise
     protected waitPaused?: (value?: void | PromiseLike<void>) => void;
     // the thread id that we were waiting for
     protected waitPausedThreadId = 0;
-    // set to true if the target was interrupted where inteneded, and should
+    // set to true if the target was interrupted where intended, and should
     // therefore be resumed after breakpoints are inserted.
     protected waitPausedNeeded = false;
     // reference count of operations requiring pausing, to make sure only the
     // first of them pauses, and the last to complete resumes
     protected pauseCount = 0;
+
+    // Pending requests to pause a thread silently (without sending stopped event).
+    // At the moment only used with pauseIfRunning().
+    protected silentPauseRequests: PendingPauseRequest[] = [];
+
     // keeps track of where in the configuration phase (between initialize event
     // and configurationDone response) we are
     protected configuringState: ConfiguringState = ConfiguringState.INITIAL;
@@ -282,16 +297,23 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
      */
     protected customResetRequest(response: DebugProtocol.Response) {
         if (this.customResetCommands) {
-            this.gdb
-                .sendCommands(this.customResetCommands)
-                .then(() => this.sendResponse(response))
-                .catch(() =>
+            this.pauseIfRunning()
+                .then(() => {
+                    // Behavior after reset very much depends on the commands used.
+                    // So, hard to make assumptions when expected state is reached.
+                    // Hence, implement stop-after-reset behavior unless commands
+                    // set running.
+                    this.gdb.sendCommands(this.customResetCommands).then(() => {
+                        this.sendResponse(response);
+                    });
+                })
+                .catch(() => {
                     this.sendErrorResponse(
                         response,
                         1,
                         'The custom reset command failed'
-                    )
-                );
+                    );
+                });
         }
     }
 
@@ -628,6 +650,43 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
         }
     }
 
+    /**
+     * Pause thread if running and suppress stopped event.
+     * In contrast to pauseIfNeeded(), there is no expectation to resume after a debugger operation.
+     *
+     * Notes:
+     *   - NOT to be called during configuration phase.
+     *   - No requireAsync, supposed to be removed from pauseIfNeeded in future.
+     */
+    protected async pauseIfRunning(): Promise<void> {
+        // Check if running before actual wait promise
+        if (!this.isRunning) {
+            // At least one thread is already stopped, good enough
+            // to execute custom reset commands.
+            return;
+        }
+        return new Promise<void>(async (resolve) => {
+            // Get current thread ID in non-stop mode
+            const currentThreadId = this.gdb.isNonStopMode()
+                ? await this.gdb.queryCurrentThreadId()
+                : undefined;
+            // Push pause request to be handled silently when stop event arrives.
+            // threadId = undefined means all threads.
+            // Note: threadId usage matches continueIfNeeded() behavior.
+            this.silentPauseRequests.push({
+                threadId: currentThreadId,
+                resolveFunc: resolve,
+            });
+            if (this.gdb.isNonStopMode()) {
+                // Send pause command to any thread (use current)
+                this.gdb.pause(currentThreadId);
+            } else {
+                this.gdb.pause();
+            }
+            // The promise resolves when pushed resolveFunc gets called.
+        });
+    }
+
     protected async dataBreakpointInfoRequest(
         response: DebugProtocol.DataBreakpointInfoResponse,
         args: DebugProtocol.DataBreakpointInfoArguments
@@ -2402,6 +2461,24 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
         return this.sendContinuedEvent(getThreadId(result));
     }
 
+    private handleSilentPauseRequests(threadId?: number): boolean {
+        // Resolve requests with exact threadId match.
+        // Note: Expecting request with threadId=undefined gets satisfied
+        // by handling a stopped event for allThreads. If this doesn't hold,
+        // below conditions should be changed to resolve all threads, i.e.
+        // also those with a defined threadId.
+        const requestsToResolve = this.silentPauseRequests.filter(
+            (request) => request.threadId === threadId
+        );
+        this.silentPauseRequests = this.silentPauseRequests.filter(
+            (request) => request.threadId !== threadId
+        );
+        requestsToResolve.forEach((request) => {
+            request.resolveFunc?.();
+        });
+        return requestsToResolve.length > 0;
+    }
+
     protected handleGDBAsync(resultClass: string, resultData: any) {
         const updateIsRunning = () => {
             this.isRunning = this.threads.length ? true : false;
@@ -2441,24 +2518,32 @@ export abstract class GDBDebugSessionBase extends LoggingDebugSession {
                             thread.lastRunToken = undefined;
                         }
                     }
-                    if (
-                        this.waitPaused &&
-                        resultData.reason === 'signal-received' &&
-                        (this.waitPausedThreadId === id ||
-                            this.waitPausedThreadId === -1)
-                    ) {
-                        suppressHandleGDBStopped = true;
+                    if (resultData.reason === 'signal-received') {
+                        if (
+                            this.waitPaused &&
+                            (this.waitPausedThreadId === id ||
+                                this.waitPausedThreadId === -1)
+                        ) {
+                            suppressHandleGDBStopped = true;
+                        }
+                        // Handle separately to avoid short-circuiting effects
+                        if (this.handleSilentPauseRequests(id)) {
+                            suppressHandleGDBStopped = true;
+                        }
                     }
                 } else {
                     for (const thread of this.threads) {
                         thread.running = false;
                         thread.lastRunToken = undefined;
                     }
-                    if (
-                        this.waitPaused &&
-                        resultData.reason === 'signal-received'
-                    ) {
-                        suppressHandleGDBStopped = true;
+                    if (resultData.reason === 'signal-received') {
+                        if (this.waitPaused) {
+                            suppressHandleGDBStopped = true;
+                        }
+                        // Handle separately to avoid short-circuiting effects
+                        if (this.handleSilentPauseRequests()) {
+                            suppressHandleGDBStopped = true;
+                        }
                     }
                 }
 
diff --git a/src/integration-tests/auxiliaryGdb.spec.ts b/src/integration-tests/auxiliaryGdb.spec.ts
@@ -57,8 +57,8 @@ describe('auxiliary gdb configuration', function () {
         if (expectToFail) {
             // Expecting launch to fail, check for correct error message
             const expectedErrorMessage = gdbNonStop
-                ? 'Cannot use auxiliaryGdb mode with gdbNonStop mode'
-                : 'AuxiliaryGdb mode requires gdbAsync to be active';
+                ? "Cannot use setting 'auxiliaryGdb' with 'gdbNonStop' mode"
+                : "Setting 'auxiliaryGdb' mode requires 'gdbAsync' to be active";
             const rejectError = await expectRejection(
                 dc.launchRequest(launchArgs)
             );
diff --git a/src/integration-tests/custom-reset.spec.ts b/src/integration-tests/custom-reset.spec.ts
@@ -12,41 +12,141 @@ import * as path from 'path';
 import { TargetLaunchRequestArguments } from '../types/session';
 import { CdtDebugClient } from './debugClient';
 import {
+    expectRejection,
     fillDefaults,
+    gdbAsync,
     isRemoteTest,
     standardBeforeEach,
     testProgramsDir,
 } from './utils';
+import { DebugProtocol } from '@vscode/debugprotocol';
+import { Runnable } from 'mocha';
+import { expect, use } from 'chai';
+import * as chaistring from 'chai-string';
+use(chaistring);
+
+const gdbtargetAdapter = 'debugTargetAdapter.js';
+const loopForeverProgram = path.join(testProgramsDir, 'loopforever');
+const commands = ['print 42'];
+const expectedResult = '$1 = 42\n';
+const customResetCommandsUnsupported = gdbAsync === false || !isRemoteTest;
+
+describe('custom reset configuration', function () {
+    let dc: CdtDebugClient;
+
+    beforeEach(async function () {
+        dc = await standardBeforeEach(gdbtargetAdapter);
+    });
+
+    afterEach(async function () {
+        if (dc) {
+            await dc.stop();
+        }
+    });
+
+    const testConnect = async (
+        launchArgs: TargetLaunchRequestArguments,
+        expectToFail: boolean
+    ) => {
+        if (expectToFail) {
+            // Expecting launch to fail, check for correct error message
+            const expectedErrorMessage =
+                "Setting 'customResetCommands' requires 'gdbAsync' to be active";
+            const rejectError = await expectRejection(
+                dc.launchRequest(launchArgs)
+            );
+            expect(rejectError.message).to.startWith(expectedErrorMessage);
+        } else {
+            // Expecting launch to succeed
+            const launchResponse = (await dc.launchRequest(
+                launchArgs
+            )) as DebugProtocol.LaunchResponse;
+            expect(launchResponse.success).to.be.true;
+        }
+    };
+
+    it('correctly validates if auxiliary gdb mode can work with other settings', async function () {
+        if (!isRemoteTest) {
+            // Only skip remote tests, gdbAsync validation tested here
+            this.skip();
+        }
+
+        const launchArgs = fillDefaults(this.test, {
+            program: loopForeverProgram,
+            customResetCommands: commands,
+        } as TargetLaunchRequestArguments);
+
+        await testConnect(launchArgs, customResetCommandsUnsupported);
+    });
+});
 
 describe('custom reset', function () {
     let dc: CdtDebugClient;
-    const emptyProgram = path.join(testProgramsDir, 'empty');
-    const commands = ['print 42'];
-    const expectedResult = '$1 = 42\n';
 
     beforeEach(async function () {
-        dc = await standardBeforeEach('debugTargetAdapter.js');
-        await dc.launchRequest(
-            fillDefaults(this.currentTest, {
-                program: emptyProgram,
-                customResetCommands: commands,
-            } as TargetLaunchRequestArguments)
-        );
+        dc = await standardBeforeEach(gdbtargetAdapter);
     });
 
     afterEach(async function () {
         await dc.stop();
     });
 
+    const completeStartup = async function (
+        testContext?: Runnable
+    ): Promise<void> {
+        // Call here instead of beforeEach so that test can be skipped without
+        // failing due to argument validation.
+        await dc.launchRequest(
+            fillDefaults(testContext, {
+                program: loopForeverProgram,
+                customResetCommands: commands,
+            } as TargetLaunchRequestArguments)
+        );
+    };
+
     it('tests sending custom reset commands', async function () {
-        if (!isRemoteTest) {
-            // command is implemented in the remote adapter but not in the local adapter
-            // so skip this test if not running remote
+        if (customResetCommandsUnsupported) {
+            // Command is implemented in GDBDebugSessionBase but deliberately documented
+            // for gdbtarget (remote) adapter only. So skip this test if not running remote
             this.skip();
         }
 
-        const event = dc.waitForOutputEvent('stdout', expectedResult);
-        await dc.customRequest('cdt-gdb-adapter/customReset');
-        await event;
+        await completeStartup(this.test);
+
+        await Promise.all([
+            dc.waitForOutputEvent('stdout', expectedResult),
+            dc.customRequest('cdt-gdb-adapter/customReset'),
+        ]);
+    });
+
+    it('stops the target if necessary before sending custom reset commands', async function () {
+        if (customResetCommandsUnsupported) {
+            // Command is implemented in GDBDebugSessionBase but deliberately documented
+            // for gdbtarget (remote) adapter only. So skip this test if not running remote.
+            // Skip if not gdbAsync, pauseIfNeeded will otherwise hang in when fetching `$_gthread`.
+            this.skip();
+        }
+
+        await completeStartup(this.test);
+
+        await dc.setFunctionBreakpointsRequest({
+            breakpoints: [{ name: 'main' }],
+        });
+        const [stoppedEvent] = await Promise.all([
+            dc.waitForEvent('stopped'),
+            dc.configurationDoneRequest(),
+        ]);
+        await dc.setFunctionBreakpointsRequest({ breakpoints: [] }); // remove function breakpoints
+
+        // Let the program run
+        await dc.continueRequest({ threadId: stoppedEvent.body.threadId });
+
+        await Promise.all([
+            dc.waitForOutputEvent('stdout', expectedResult), // wait stdout event
+            dc.customRequest('cdt-gdb-adapter/customReset'),
+        ]);
+
+        // Would throw if it wasn't stopped
+        await dc.stepInRequest({ threadId: stoppedEvent.body.threadId });
     });
 });
diff --git a/src/web/GDBTargetDebugSession.ts b/src/web/GDBTargetDebugSession.ts

Original file line number	Diff line number	Diff line change
`@@ -187,15 +187,20 @@ export class GDBTargetDebugSession extends GDBDebugSession {`
`187`	`187`	`// Limitations for auxiliary GDB mode`
`188`	`188`	`if (args.gdbNonStop) {`
`189`	`189`	`throw new Error(`
`190`		`- 'Cannot use auxiliaryGdb mode with gdbNonStop mode'`
	`190`	`+ "Cannot use setting 'auxiliaryGdb' with 'gdbNonStop' mode"`
`191`	`191`	`);`
`192`	`192`	`}`
`193`	`193`	`if (args.gdbAsync === false) {`
`194`	`194`	`throw new Error(`
`195`		`- 'AuxiliaryGdb mode requires gdbAsync to be active'`
	`195`	`+ "Setting 'auxiliaryGdb' mode requires 'gdbAsync' to be active"`
`196`	`196`	`);`
`197`	`197`	`}`
`198`	`198`	`}`
	`199`	`+ if (args.customResetCommands?.length && args.gdbAsync === false) {`
	`200`	`+ throw new Error(`
	`201`	`+ "Setting 'customResetCommands' requires 'gdbAsync' to be active"`
	`202`	`+ );`
	`203`	`+ }`
`199`	`204`	`}`
`200`	`205`
`201`	`206`	`protected override async setupCommonLoggerAndBackends(`