Skip to content

Commit 4f29a51

Browse files
authored
each component instance has it's own session information (#824)
* each component instance has it's own session information * removing residual functions * changing finally to catch to print error message * updating tests to accommodate change * throwing errors if payload session is different than that of active session
1 parent fb1c762 commit 4f29a51

8 files changed

Lines changed: 24 additions & 79 deletions

src/views/component-viewer/component-viewer-instance.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,6 @@ export class ComponentViewerInstance {
104104
return `${text}, Time: ${timeUsage} ms, Mem: ${memUsage}, Mem Increase: ${memIncrease} MB, (Total: ${memCurrent} MB)`;
105105
}
106106

107-
public updateActiveSession(debugSession: GDBTargetDebugSession): void {
108-
this._scvdEvalContext?.updateActiveSession(debugSession);
109-
}
110-
111107
public async readModel(filename: URI, debugSession: GDBTargetDebugSession, debugTracker: GDBTargetDebugTracker): Promise<void> {
112108
const stats: string[] = [];
113109
this._fileKey = ComponentViewerInstance.getFileKey(filename);

src/views/component-viewer/component-viewer-main.ts

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,7 @@ export class ComponentViewer {
171171
private async handleOnStackTrace(session: GDBTargetDebugSession): Promise<void> {
172172
// Clear active session if it is NOT the one being stopped
173173
if (this._activeSession?.session.id !== session.session.id) {
174-
this._activeSession = undefined;
175-
return;
174+
throw new Error(`Component Viewer: Received stack trace event for session ${session.session.id} while active session is ${this._activeSession?.session.id}`);
176175
}
177176
// Update component viewer instance(s)
178177
this.schedulePendingUpdate('stackTrace');
@@ -182,8 +181,7 @@ export class ComponentViewer {
182181
// If the active session is not the one being updated, update it.
183182
// This can happen when a session is started and stack trace/item events are emitted before the session is set as active in the component viewer.
184183
if (this._activeSession?.session.id !== session.session.id) {
185-
this._activeSession = session;
186-
this._instances.forEach((instance) => instance.componentViewerInstance.updateActiveSession(session));
184+
throw new Error(`Component Viewer: Received stack item changed event for session ${session.session.id} while active session is ${this._activeSession?.session.id}`);
187185
}
188186
// Update component viewer instance(s)
189187
this.schedulePendingUpdate('stackItemChanged');
@@ -226,11 +224,6 @@ export class ComponentViewer {
226224
private async handleOnDidChangeActiveDebugSession(session: GDBTargetDebugSession | undefined): Promise<void> {
227225
// Update debug session
228226
this._activeSession = session;
229-
if (session === undefined) {
230-
return;
231-
}
232-
// update active debug session for all instances
233-
this._instances.forEach((instance) => instance.componentViewerInstance.updateActiveSession(session));
234227
}
235228

236229
private schedulePendingUpdate(updateReason: fifoUpdateReason): void {
@@ -253,9 +246,8 @@ export class ComponentViewer {
253246
this._pendingUpdate = false;
254247
try {
255248
await this.updateInstances(updateReason);
256-
} finally {
257-
this._runningUpdate = false;
258-
//logger.error('Component Viewer: Error during update');
249+
} catch (error) {
250+
componentViewerLogger.error(`Component Viewer: Error during update - ${(error as Error).message}`);
259251
}
260252
}
261253
this._runningUpdate = false;
@@ -273,11 +265,12 @@ export class ComponentViewer {
273265
}
274266
perf?.resetBackendStats();
275267
perf?.resetUiStats();
268+
const activeSessionID = this._activeSession.session.id;
276269
const roots: ScvdGuiInterface[] = [];
277270
for (const instance of this._instances) {
278271
// Check if instance belongs to the active session, if not skip it and clear its data from the tree view.
279272
// However, lockedState should be maintained.
280-
if (instance.sessionId !== this._activeSession.session.id) {
273+
if (instance.sessionId !== activeSessionID) {
281274
instance.componentViewerInstance.getGuiTree()?.forEach(root => root.clear());
282275
continue;
283276
}

src/views/component-viewer/scvd-debug-target.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,6 @@ export class ScvdDebugTarget {
106106
this.symbolCaches.clearAll();
107107
}
108108

109-
public setActiveSession(session: GDBTargetDebugSession): void {
110-
componentViewerLogger.debug('updating active session');
111-
this.activeSession = session;
112-
this.targetAccess.setActiveSession(session);
113-
}
114-
115109
public async beginUpdateCycle(): Promise<void> {
116110
componentViewerLogger.debug('reset read stats and prime cache');
117111
targetReadStats?.reset();

src/views/component-viewer/scvd-eval-context.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,6 @@ export class ScvdEvalContext {
125125
})();
126126
}
127127

128-
public updateActiveSession(debugSession: GDBTargetDebugSession): void {
129-
this._debugTarget.setActiveSession(debugSession);
130-
}
131-
132128
public setIntegerModelKind(kind: IntegerModelKind | undefined): void {
133129
if (!kind) {
134130
return;

src/views/component-viewer/test/integration/scvd-debug-target.test.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,6 @@ describe('scvd-debug-target', () => {
302302
const target = new ScvdDebugTarget();
303303
target.init(session, tracker);
304304

305-
const otherSession = { session: { id: 'sess-2' } } as unknown as GDBTargetDebugSession;
306-
target.setActiveSession(otherSession);
307-
expect(accessMock.setActiveSession).toHaveBeenCalledWith(otherSession);
308-
309305
(target as unknown as { targetReadCache: TargetReadCache | undefined }).targetReadCache = undefined;
310306
await target.beginUpdateCycle();
311307

src/views/component-viewer/test/unit/component-viewer-instance.test.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -267,20 +267,6 @@ describe('ComponentViewerInstance', () => {
267267
expect(second).toContain('Mem Increase:');
268268
});
269269

270-
it('stores scvd eval context and forwards active session updates', () => {
271-
const instance = new ComponentViewerInstance();
272-
const updateActiveSession = jest.fn();
273-
const evalContext = { updateActiveSession } as unknown as ScvdEvalContext;
274-
const debugSession = {} as unknown as GDBTargetDebugSession;
275-
276-
expect(instance.scvdEvalContext).toBeUndefined();
277-
instance.scvdEvalContext = evalContext;
278-
expect(instance.scvdEvalContext).toBe(evalContext);
279-
280-
instance.updateActiveSession(debugSession);
281-
expect(updateActiveSession).toHaveBeenCalledWith(debugSession);
282-
});
283-
284270
it('reuses existing file key for the same path', () => {
285271
const keyMap = (ComponentViewerInstance as unknown as { _fileKeysByPath: Map<string, string> })._fileKeysByPath;
286272
const countMap = (ComponentViewerInstance as unknown as { _fileKeyCounts: Map<string, number> })._fileKeyCounts;

src/views/component-viewer/test/unit/component-viewer-main.test.ts

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,11 @@ describe('ComponentViewer', () => {
281281

282282
(controller as unknown as { _activeSession?: Session })._activeSession = session;
283283
await tracker.callbacks.stackTrace?.({ session });
284-
await tracker.callbacks.stackTrace?.({ session: otherSession });
284+
await expect(tracker.callbacks.stackTrace?.({ session: otherSession }) as Promise<void>).rejects.toThrow(
285+
'Component Viewer: Received stack trace event for session s2 while active session is s1'
286+
);
287+
expect((controller as unknown as { _activeSession?: Session })._activeSession).toBe(session);
285288

286-
// stackTrace from a different session clears active session
287-
expect((controller as unknown as { _activeSession?: Session })._activeSession).toBeUndefined();
288-
289-
await tracker.callbacks.activeStackItem?.({ session: otherSession });
290-
expect((controller as unknown as { _activeSession?: Session })._activeSession).toBe(otherSession);
291289

292290
(controller as unknown as { _activeSession?: Session })._activeSession = session;
293291
await tracker.callbacks.willStop?.(session);
@@ -336,32 +334,28 @@ describe('ComponentViewer', () => {
336334
expect((controller as unknown as { _instances: unknown[] })._instances).toHaveLength(0);
337335
});
338336

339-
it('updates active session and instances on stack item change', async () => {
337+
it('updates instances on stack item change', async () => {
340338
const controller = new ComponentViewer(extensionContextFactory());
341339
const sessionA = makeSession('s1');
342-
const sessionB = makeSession('s2');
343-
const updateSpy = jest.fn();
340+
const updateSpy = jest.fn().mockResolvedValue(undefined);
344341

345342
(controller as unknown as { _activeSession?: Session })._activeSession = sessionA;
346343
(controller as unknown as { _instances: ComponentViewerInstancesWrapper[] })._instances = [
347344
{
348-
componentViewerInstance: { updateActiveSession: updateSpy } as unknown as ComponentViewerInstancesWrapper['componentViewerInstance'],
345+
componentViewerInstance: { update: updateSpy, getGuiTree: jest.fn(() => []) } as unknown as ComponentViewerInstancesWrapper['componentViewerInstance'],
349346
lockState: false,
350347
sessionId: 's1',
351348
},
352349
];
353350

354-
const scheduleSpy = jest.spyOn(
355-
controller as unknown as { schedulePendingUpdate: (reason: fifoUpdateReason) => void },
356-
'schedulePendingUpdate'
357-
).mockImplementation(() => undefined);
358-
359351
const handleOnStackItemChanged = (controller as unknown as { handleOnStackItemChanged: (s: Session) => Promise<void> }).handleOnStackItemChanged.bind(controller);
360-
await handleOnStackItemChanged(sessionB);
352+
jest.useFakeTimers();
353+
await handleOnStackItemChanged(sessionA);
354+
jest.advanceTimersByTime(200);
355+
await Promise.resolve();
361356

362-
expect((controller as unknown as { _activeSession?: Session })._activeSession).toBe(sessionB);
363-
expect(updateSpy).toHaveBeenCalledWith(sessionB);
364-
expect(scheduleSpy).toHaveBeenCalledWith('stackItemChanged');
357+
expect(updateSpy).toHaveBeenCalled();
358+
jest.useRealTimers();
365359
});
366360

367361
it('does not update active session when stack item matches the active session', async () => {
@@ -629,17 +623,17 @@ describe('ComponentViewer', () => {
629623
expect(updateInstances).toHaveBeenCalledWith('stackTrace');
630624
});
631625

632-
it('propagates errors during a coalescing update', async () => {
626+
it('swallows errors during a coalescing update', async () => {
633627
const controller = new ComponentViewer(extensionContextFactory());
634628
(controller as unknown as { _pendingUpdate: boolean })._pendingUpdate = true;
635629
(controller as unknown as { _runningUpdate: boolean })._runningUpdate = false;
636-
(controller as unknown as { updateInstances: (reason: fifoUpdateReason) => Promise<void> }).updateInstances = jest
637-
.fn()
630+
const updateInstances = jest.spyOn(controller as unknown as { updateInstances: (reason: fifoUpdateReason) => Promise<void> }, 'updateInstances')
638631
.mockRejectedValue(new Error('fail'));
639632
const runUpdate = (controller as unknown as { runUpdate: (reason: fifoUpdateReason) => Promise<void> }).runUpdate.bind(controller);
640633

641-
await expect(runUpdate('stackTrace')).rejects.toThrow('fail');
642-
// Clears running state if runUpdate throws
634+
await expect(runUpdate('stackTrace')).resolves.toBeUndefined();
635+
expect(updateInstances).toHaveBeenCalledWith('stackTrace');
636+
// Clears running state after runUpdate completes
643637
expect((controller as unknown as { _runningUpdate: boolean })._runningUpdate).toBe(false);
644638
});
645639

src/views/component-viewer/test/unit/scvd-eval-context.test.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,6 @@ describe('ScvdEvalContext', () => {
108108
expect(debugTarget.init).toHaveBeenCalledWith(fakeSession, fakeTracker);
109109
});
110110

111-
it('updates active session on debug target', () => {
112-
const { viewer } = buildViewerWithObject();
113-
const ctx = new ScvdEvalContext(viewer);
114-
const debugTarget = ctx.getExecutionContext().debugTarget;
115-
const spy = jest.spyOn(debugTarget, 'setActiveSession').mockImplementation(() => undefined as unknown as void);
116-
const fakeSession = {} as unknown as Parameters<typeof debugTarget.setActiveSession>[0];
117-
ctx.updateActiveSession(fakeSession);
118-
expect(spy).toHaveBeenCalledWith(fakeSession);
119-
});
120-
121111
it('updates integer model when setIntegerModelKind is provided', () => {
122112
const { viewer } = buildViewerWithObject();
123113
const ctx = new ScvdEvalContext(viewer);

0 commit comments

Comments
 (0)