Skip reading CPU frequency from 'SystemCoreClock' after failure. (#658)

* Skip reading CPU frequency from 'SystemCoreClock' after failure. Retries after CPU Time Information reset.
* Initial CPU frequency update if target not stopped after connect
* Fix logic to keep previous frequency if getting undefined

Author: jreineckearm

src/debug-session/gdbtarget-debug-session.ts

@@ -25,14 +25,14 @@ import { PeriodicRefreshTimer } from './periodic-refresh-timer';
  */
 export class GDBTargetDebugSession {
     public readonly refreshTimer: PeriodicRefreshTimer<GDBTargetDebugSession>;
+    public readonly canAccessWhileRunning: boolean;
     private _cbuildRun: CbuildRunReader|undefined;
     private _cbuildRunParsePromise: Promise<void>|undefined;
 
     constructor(public session: vscode.DebugSession) {
         this.refreshTimer = new PeriodicRefreshTimer(this);
-        if (this.session.configuration.type === 'gdbtarget') {
-            this.refreshTimer.enabled = this.session.configuration['auxiliaryGdb'] === true;
-        }
+        this.canAccessWhileRunning = this.session.configuration.type === 'gdbtarget' && this.session.configuration['auxiliaryGdb'] === true;
+        this.refreshTimer.enabled = this.canAccessWhileRunning;
     }
 
     public async getCbuildRun(): Promise<CbuildRunReader|undefined> {

src/features/cpu-states/cpu-states.test.ts

@@ -188,9 +188,17 @@ describe('CpuStates', () => {
             // eslint-disable-next-line @typescript-eslint/no-explicit-any
             (tracker as any)._onConnected.fire(gdbtargetDebugSession);
             await waitForMs(0);
+            // Bring into stopped state
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+            (tracker as any)._onStopped.fire(createStoppedEvent(gdbtargetDebugSession, 'breakpoint', 0));
+            await waitForMs(0);
         });
 
         it('handles running state correctly', async () => {
+            // Set running
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+            (tracker as any)._onContinued.fire(createContinuedEvent(gdbtargetDebugSession, 0));
+            await waitForMs(0);
             // Considered running after connection
             expect(cpuStates.activeCpuStates?.isRunning).toEqual(true);
             // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -329,6 +337,7 @@ describe('CpuStates', () => {
             (debugSession.customRequest as jest.Mock).mockRejectedValueOnce(new Error('test error'));
             await cpuStates.updateFrequency();
             expect(cpuStates.activeCpuStates).toBeDefined();
+            expect(cpuStates.activeCpuStates?.skipFrequencyUpdate).toEqual(true);
             expect(cpuStates.activeCpuStates?.frequency).toBeUndefined();
             expect(cpuStates.activeCpuStates?.statesHistory.frequency).toBeUndefined();
             expect(await cpuStates.getActiveTimeString()).toEqual(' 0 states');
@@ -338,6 +347,8 @@ describe('CpuStates', () => {
             (debugSession.customRequest as jest.Mock).mockReturnValueOnce({ result: 'not a number' });
             await cpuStates.updateFrequency();
             expect(cpuStates.activeCpuStates).toBeDefined();
+            // Not considered as severe error yet, so not skipping frequency updates
+            expect(cpuStates.activeCpuStates?.skipFrequencyUpdate).toEqual(false);
             expect(cpuStates.activeCpuStates?.frequency).toBeUndefined();
             expect(cpuStates.activeCpuStates?.statesHistory.frequency).toBeUndefined();
             expect(await cpuStates.getActiveTimeString()).toEqual(' 0 states');
@@ -347,11 +358,40 @@ describe('CpuStates', () => {
             (debugSession.customRequest as jest.Mock).mockReturnValueOnce({ result: '12000000' });
             await cpuStates.updateFrequency();
             expect(cpuStates.activeCpuStates).toBeDefined();
+            expect(cpuStates.activeCpuStates?.skipFrequencyUpdate).toEqual(false);
             expect(cpuStates.activeCpuStates?.frequency).toEqual(12000000);
             expect(cpuStates.activeCpuStates?.statesHistory.frequency).toEqual(12000000);
             expect(await cpuStates.getActiveTimeString()).toEqual(' 0ns');
         });
 
+        it('skips updating frequency after getting SystemCoreClock throws until next CPU states reset', async () => {
+            expect(cpuStates.activeCpuStates?.skipFrequencyUpdate).toEqual(false);
+            (debugSession.customRequest as jest.Mock).mockRejectedValueOnce(new Error('test error'));
+            // First update causing CPU states to skip further updates, time string in states
+            await cpuStates.updateFrequency();
+            expect(cpuStates.activeCpuStates).toBeDefined();
+            expect(cpuStates.activeCpuStates?.frequency).toBeUndefined();
+            expect(cpuStates.activeCpuStates?.statesHistory.frequency).toBeUndefined();
+            expect(cpuStates.activeCpuStates?.skipFrequencyUpdate).toEqual(true);
+            expect(await cpuStates.getActiveTimeString()).toEqual(' 0 states');
+            // Second update, valid frequency but still skipping frequency update, time string still in states
+            (debugSession.customRequest as jest.Mock).mockResolvedValueOnce({ result: '12000000' });
+            await cpuStates.updateFrequency();
+            expect(cpuStates.activeCpuStates?.frequency).toBeUndefined();
+            expect(cpuStates.activeCpuStates?.skipFrequencyUpdate).toEqual(true);
+            expect(await cpuStates.getActiveTimeString()).toEqual(' 0 states');
+            // Reset states, skipFrequencyUpdate reset to false
+            cpuStates.resetStatesHistory();
+            expect(cpuStates.activeCpuStates?.skipFrequencyUpdate).toEqual(false);
+            // Now update again
+            // Third update, valid frequency, time string now in time
+            (debugSession.customRequest as jest.Mock).mockResolvedValueOnce({ result: '12000000' });
+            await cpuStates.updateFrequency();
+            expect(cpuStates.activeCpuStates?.frequency).toEqual(12000000);
+            expect(cpuStates.activeCpuStates?.skipFrequencyUpdate).toEqual(false);
+            expect(await cpuStates.getActiveTimeString()).toEqual(' 0ns');
+        });
+
         it('assigns frame location to captured stop point based on threadId match', async () => {
             const debugConsoleOutput: string[] = [];
             (vscode.debug.activeDebugConsole.appendLine as jest.Mock).mockImplementation(line => debugConsoleOutput.push(line));

src/features/cpu-states/cpu-states.ts

@@ -26,6 +26,7 @@ import { GDBTargetDebugSession } from '../../debug-session/gdbtarget-debug-sessi
 import { CpuStatesHistory } from './cpu-states-history';
 import { calculateTime, extractPname } from '../../utils';
 import { GDBTargetConfiguration } from '../../debug-configuration';
+import { logger } from '../../logger';
 
 // Architecturally defined registers (M-profile)
 const DWT_CTRL_ADDRESS = 0xE0001000;
@@ -41,6 +42,7 @@ interface SessionCpuStates {
     statesHistory: CpuStatesHistory;
     isRunning: boolean;
     hasStates: boolean|undefined;
+    skipFrequencyUpdate: boolean;
 }
 
 export class CpuStates {
@@ -75,7 +77,8 @@ export class CpuStates {
             frequency: undefined,
             statesHistory: new CpuStatesHistory(extractPname(session.session.name)),
             isRunning: true,
-            hasStates: undefined
+            hasStates: undefined,
+            skipFrequencyUpdate: false
         };
         this.sessionCpuStates.set(session.session.id, states);
         session.refreshTimer.onRefresh(async (refreshSession) => this.handlePeriodicRefresh(refreshSession));
@@ -216,6 +219,10 @@ export class CpuStates {
     protected async getFrequency(): Promise<number|undefined> {
         const result = await this.activeSession?.evaluateGlobalExpression('SystemCoreClock');
         if (typeof result == 'string') {
+            if (this.activeCpuStates) {
+                // Do not retry until reset of CPU Time information
+                this.activeCpuStates.skipFrequencyUpdate = true;
+            }
             return undefined;
         }
         const frequencyString = result?.result.match(/\d+/) ? result.result : undefined;
@@ -252,11 +259,7 @@ export class CpuStates {
             return undefined;
         }
         const pname = await this.getActivePname();
-        if (!this.activeCpuStates.isRunning) {
-            // Only update frequency while stopped. User previous otherwise
-            // to avoid switching between states and time display.
-            await this.updateFrequency();
-        }
+        await this.updateFrequency();
         const cpuName = pname ? ` ${pname} ` : '';
         if (!this.activeHasStates()) {
             return `${cpuName} N/A`;
@@ -268,11 +271,25 @@ export class CpuStates {
     }
 
     public async updateFrequency(): Promise<void> {
+        // Only update frequency while stopped. Use previous otherwise
+        // to avoid switching between states and time display.
         const states = this.activeCpuStates;
-        if (!states) {
+        if (!states || states.skipFrequencyUpdate) {
+            // No states or frequency update disabled
             return;
         }
+        if (this.activeCpuStates.isRunning && (this.activeCpuStates.frequency !== undefined || !this.activeSession?.canAccessWhileRunning)) {
+            // Skip update while running if we already have a frequency (update after next stop to avoid jumpy time display),
+            // or if session does not support access while running.
+            return;
+        }
+        // Update frequency if stopped or if initial update while running and access while running supported.
         const frequency = await this.getFrequency();
+        if (frequency === undefined) {
+            // Keep previous frequency value to avoid toggling between states and time display if there was a frequency before.
+            logger.debug(`CPU States: Unable to read frequency from target, keeping previous value '${states.frequency}' (Session: '${this.activeSession?.session.name})'`);
+            return;
+        }
         states.frequency = frequency;
         states.statesHistory.frequency = frequency;
     }
@@ -300,6 +317,7 @@ export class CpuStates {
         }
         states.statesHistory.resetHistory();
         states.states = BigInt(0);
+        states.skipFrequencyUpdate = false;
         this._onRefresh.fire(0);
     }
 };