Skip to content

Commit aa79bd7

Browse files
roblourensroblourens
authored
Better fix for previous candidate debug issue (microsoft#182930)
Bring back the async queue for handling debug messages, but be sure that refreshTopOfCallstack will always resolve its promises and won't make the queue stuck Fix microsoft#181966
1 parent 42dc9f0 commit aa79bd7

File tree

3 files changed

+160
-94
lines changed

3 files changed

+160
-94
lines changed

‎src/vs/workbench/contrib/debug/browser/debugSession.ts

Lines changed: 81 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -975,76 +975,81 @@ export class DebugSession implements IDebugSession {
975975
}
976976
}));
977977

978+
const statusQueue = new Queue<void>();
978979
this.rawListeners.push(this.raw.onDidStop(async event => {
979-
this.passFocusScheduler.cancel();
980-
this.stoppedDetails.push(event.body);
981-
await this.fetchThreads(event.body);
982-
// If the focus for the current session is on a non-existent thread, clear the focus.
983-
const focusedThread = this.debugService.getViewModel().focusedThread;
984-
const focusedThreadDoesNotExist = focusedThread !== undefined && focusedThread.session === this && !this.threads.has(focusedThread.threadId);
985-
if (focusedThreadDoesNotExist) {
986-
this.debugService.focusStackFrame(undefined, undefined);
987-
}
988-
const thread = typeof event.body.threadId === 'number' ? this.getThread(event.body.threadId) : undefined;
989-
if (thread) {
990-
// Call fetch call stack twice, the first only return the top stack frame.
991-
// Second retrieves the rest of the call stack. For performance reasons #25605
992-
const promises = this.model.refreshTopOfCallstack(<Thread>thread);
993-
const focus = async () => {
994-
if (focusedThreadDoesNotExist || (!event.body.preserveFocusHint && thread.getCallStack().length)) {
995-
const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame;
996-
if (!focusedStackFrame || focusedStackFrame.thread.session === this) {
997-
// Only take focus if nothing is focused, or if the focus is already on the current session
998-
const preserveFocus = !this.configurationService.getValue<IDebugConfiguration>('debug').focusEditorOnBreak;
999-
await this.debugService.focusStackFrame(undefined, thread, undefined, { preserveFocus });
1000-
}
1001-
1002-
if (thread.stoppedDetails) {
1003-
if (thread.stoppedDetails.reason === 'breakpoint' && this.configurationService.getValue<IDebugConfiguration>('debug').openDebug === 'openOnDebugBreak' && !this.suppressDebugView) {
1004-
await this.paneCompositeService.openPaneComposite(VIEWLET_ID, ViewContainerLocation.Sidebar);
980+
statusQueue.queue(async () => {
981+
this.passFocusScheduler.cancel();
982+
this.stoppedDetails.push(event.body);
983+
await this.fetchThreads(event.body);
984+
// If the focus for the current session is on a non-existent thread, clear the focus.
985+
const focusedThread = this.debugService.getViewModel().focusedThread;
986+
const focusedThreadDoesNotExist = focusedThread !== undefined && focusedThread.session === this && !this.threads.has(focusedThread.threadId);
987+
if (focusedThreadDoesNotExist) {
988+
this.debugService.focusStackFrame(undefined, undefined);
989+
}
990+
const thread = typeof event.body.threadId === 'number' ? this.getThread(event.body.threadId) : undefined;
991+
if (thread) {
992+
// Call fetch call stack twice, the first only return the top stack frame.
993+
// Second retrieves the rest of the call stack. For performance reasons #25605
994+
const promises = this.model.refreshTopOfCallstack(<Thread>thread);
995+
const focus = async () => {
996+
if (focusedThreadDoesNotExist || (!event.body.preserveFocusHint && thread.getCallStack().length)) {
997+
const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame;
998+
if (!focusedStackFrame || focusedStackFrame.thread.session === this) {
999+
// Only take focus if nothing is focused, or if the focus is already on the current session
1000+
const preserveFocus = !this.configurationService.getValue<IDebugConfiguration>('debug').focusEditorOnBreak;
1001+
await this.debugService.focusStackFrame(undefined, thread, undefined, { preserveFocus });
10051002
}
10061003

1007-
if (this.configurationService.getValue<IDebugConfiguration>('debug').focusWindowOnBreak && !this.workbenchEnvironmentService.extensionTestsLocationURI) {
1008-
await this.hostService.focus({ force: true /* Application may not be active */ });
1004+
if (thread.stoppedDetails) {
1005+
if (thread.stoppedDetails.reason === 'breakpoint' && this.configurationService.getValue<IDebugConfiguration>('debug').openDebug === 'openOnDebugBreak' && !this.suppressDebugView) {
1006+
await this.paneCompositeService.openPaneComposite(VIEWLET_ID, ViewContainerLocation.Sidebar);
1007+
}
1008+
1009+
if (this.configurationService.getValue<IDebugConfiguration>('debug').focusWindowOnBreak && !this.workbenchEnvironmentService.extensionTestsLocationURI) {
1010+
await this.hostService.focus({ force: true /* Application may not be active */ });
1011+
}
10091012
}
10101013
}
1011-
}
1012-
};
1013-
1014-
await promises.topCallStack;
1015-
focus();
1016-
await promises.wholeCallStack;
1017-
const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame;
1018-
if (!focusedStackFrame || !focusedStackFrame.source || focusedStackFrame.source.presentationHint === 'deemphasize' || focusedStackFrame.presentationHint === 'deemphasize') {
1019-
// The top stack frame can be deemphesized so try to focus again #68616
1014+
};
1015+
1016+
await promises.topCallStack;
10201017
focus();
1018+
await promises.wholeCallStack;
1019+
const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame;
1020+
if (!focusedStackFrame || !focusedStackFrame.source || focusedStackFrame.source.presentationHint === 'deemphasize' || focusedStackFrame.presentationHint === 'deemphasize') {
1021+
// The top stack frame can be deemphesized so try to focus again #68616
1022+
focus();
1023+
}
10211024
}
1022-
}
1023-
this._onDidChangeState.fire();
1025+
this._onDidChangeState.fire();
1026+
});
10241027
}));
10251028

10261029
this.rawListeners.push(this.raw.onDidThread(event => {
1027-
if (event.body.reason === 'started') {
1028-
// debounce to reduce threadsRequest frequency and improve performance
1029-
if (!this.fetchThreadsScheduler) {
1030-
this.fetchThreadsScheduler = new RunOnceScheduler(() => {
1031-
this.fetchThreads();
1032-
}, 100);
1033-
this.rawListeners.push(this.fetchThreadsScheduler);
1034-
}
1035-
if (!this.fetchThreadsScheduler.isScheduled()) {
1036-
this.fetchThreadsScheduler.schedule();
1037-
}
1038-
} else if (event.body.reason === 'exited') {
1039-
this.model.clearThreads(this.getId(), true, event.body.threadId);
1040-
const viewModel = this.debugService.getViewModel();
1041-
const focusedThread = viewModel.focusedThread;
1042-
this.passFocusScheduler.cancel();
1043-
if (focusedThread && event.body.threadId === focusedThread.threadId) {
1044-
// De-focus the thread in case it was focused
1045-
this.debugService.focusStackFrame(undefined, undefined, viewModel.focusedSession, { explicit: false });
1030+
statusQueue.queue(async () => {
1031+
if (event.body.reason === 'started') {
1032+
// debounce to reduce threadsRequest frequency and improve performance
1033+
if (!this.fetchThreadsScheduler) {
1034+
this.fetchThreadsScheduler = new RunOnceScheduler(() => {
1035+
this.fetchThreads();
1036+
}, 100);
1037+
this.rawListeners.push(this.fetchThreadsScheduler);
1038+
}
1039+
if (!this.fetchThreadsScheduler.isScheduled()) {
1040+
this.fetchThreadsScheduler.schedule();
1041+
}
1042+
} else if (event.body.reason === 'exited') {
1043+
this.model.clearThreads(this.getId(), true, event.body.threadId);
1044+
const viewModel = this.debugService.getViewModel();
1045+
const focusedThread = viewModel.focusedThread;
1046+
this.passFocusScheduler.cancel();
1047+
if (focusedThread && event.body.threadId === focusedThread.threadId) {
1048+
// De-focus the thread in case it was focused
1049+
this.debugService.focusStackFrame(undefined, undefined, viewModel.focusedSession, { explicit: false });
1050+
}
10461051
}
1047-
}
1052+
});
10481053
}));
10491054

10501055
this.rawListeners.push(this.raw.onDidTerminateDebugee(async event => {
@@ -1057,21 +1062,23 @@ export class DebugSession implements IDebugSession {
10571062
}));
10581063

10591064
this.rawListeners.push(this.raw.onDidContinued(event => {
1060-
const threadId = event.body.allThreadsContinued !== false ? undefined : event.body.threadId;
1061-
if (typeof threadId === 'number') {
1062-
this.stoppedDetails = this.stoppedDetails.filter(sd => sd.threadId !== threadId);
1063-
const tokens = this.cancellationMap.get(threadId);
1064-
this.cancellationMap.delete(threadId);
1065-
tokens?.forEach(t => t.cancel());
1066-
} else {
1067-
this.stoppedDetails = [];
1068-
this.cancelAllRequests();
1069-
}
1070-
this.lastContinuedThreadId = threadId;
1071-
// We need to pass focus to other sessions / threads with a timeout in case a quick stop event occurs #130321
1072-
this.passFocusScheduler.schedule();
1073-
this.model.clearThreads(this.getId(), false, threadId);
1074-
this._onDidChangeState.fire();
1065+
statusQueue.queue(async () => {
1066+
const threadId = event.body.allThreadsContinued !== false ? undefined : event.body.threadId;
1067+
if (typeof threadId === 'number') {
1068+
this.stoppedDetails = this.stoppedDetails.filter(sd => sd.threadId !== threadId);
1069+
const tokens = this.cancellationMap.get(threadId);
1070+
this.cancellationMap.delete(threadId);
1071+
tokens?.forEach(t => t.cancel());
1072+
} else {
1073+
this.stoppedDetails = [];
1074+
this.cancelAllRequests();
1075+
}
1076+
this.lastContinuedThreadId = threadId;
1077+
// We need to pass focus to other sessions / threads with a timeout in case a quick stop event occurs #130321
1078+
this.passFocusScheduler.schedule();
1079+
this.model.clearThreads(this.getId(), false, threadId);
1080+
this._onDidChangeState.fire();
1081+
});
10751082
}));
10761083

10771084
const outputQueue = new Queue<void>();

‎src/vs/workbench/contrib/debug/common/debugModel.ts

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { distinct, lastIndex } from 'vs/base/common/arrays';
7-
import { RunOnceScheduler } from 'vs/base/common/async';
7+
import { DeferredPromise, RunOnceScheduler } from 'vs/base/common/async';
88
import { decodeBase64, encodeBase64, VSBuffer } from 'vs/base/common/buffer';
99
import { CancellationTokenSource } from 'vs/base/common/cancellation';
1010
import { stringHash } from 'vs/base/common/hash';
@@ -1176,7 +1176,7 @@ export class ThreadAndSessionIds implements ITreeElement {
11761176
export class DebugModel implements IDebugModel {
11771177

11781178
private sessions: IDebugSession[];
1179-
private schedulers = new Map<string, RunOnceScheduler>();
1179+
private schedulers = new Map<string, { scheduler: RunOnceScheduler; completeDeferred: DeferredPromise<void> }>();
11801180
private breakpointsActivated = true;
11811181
private readonly _onDidChangeBreakpoints = new Emitter<IBreakpointsChangeEvent | undefined>();
11821182
private readonly _onDidChangeCallStack = new Emitter<void>();
@@ -1274,7 +1274,10 @@ export class DebugModel implements IDebugModel {
12741274

12751275
clearThreads(id: string, removeThreads: boolean, reference: number | undefined = undefined): void {
12761276
const session = this.sessions.find(p => p.getId() === id);
1277-
this.schedulers.forEach(scheduler => scheduler.dispose());
1277+
this.schedulers.forEach(entry => {
1278+
entry.scheduler.dispose();
1279+
entry.completeDeferred.complete();
1280+
});
12781281
this.schedulers.clear();
12791282

12801283
if (session) {
@@ -1314,24 +1317,32 @@ export class DebugModel implements IDebugModel {
13141317
const wholeCallStack = new Promise<void>((c, e) => {
13151318
topCallStack = thread.fetchCallStack(1).then(() => {
13161319
if (!this.schedulers.has(thread.getId())) {
1317-
this.schedulers.set(thread.getId(), new RunOnceScheduler(() => {
1318-
thread.fetchCallStack(19).then(() => {
1319-
const stale = thread.getStaleCallStack();
1320-
const current = thread.getCallStack();
1321-
let bottomOfCallStackChanged = stale.length !== current.length;
1322-
for (let i = 1; i < stale.length && !bottomOfCallStackChanged; i++) {
1323-
bottomOfCallStackChanged = !stale[i].equals(current[i]);
1324-
}
1325-
1326-
if (bottomOfCallStackChanged) {
1327-
this._onDidChangeCallStack.fire();
1328-
}
1329-
c();
1330-
});
1331-
}, 420));
1320+
const deferred = new DeferredPromise<void>();
1321+
this.schedulers.set(thread.getId(), {
1322+
completeDeferred: deferred,
1323+
scheduler: new RunOnceScheduler(() => {
1324+
thread.fetchCallStack(19).then(() => {
1325+
const stale = thread.getStaleCallStack();
1326+
const current = thread.getCallStack();
1327+
let bottomOfCallStackChanged = stale.length !== current.length;
1328+
for (let i = 1; i < stale.length && !bottomOfCallStackChanged; i++) {
1329+
bottomOfCallStackChanged = !stale[i].equals(current[i]);
1330+
}
1331+
1332+
if (bottomOfCallStackChanged) {
1333+
this._onDidChangeCallStack.fire();
1334+
}
1335+
}).finally(() => {
1336+
deferred.complete();
1337+
this.schedulers.delete(thread.getId());
1338+
});
1339+
}, 420)
1340+
});
13321341
}
13331342

1334-
this.schedulers.get(thread.getId())!.schedule();
1343+
const entry = this.schedulers.get(thread.getId())!;
1344+
entry.scheduler.schedule();
1345+
entry.completeDeferred.p.then(c, e);
13351346
this._onDidChangeCallStack.fire();
13361347
});
13371348
});

‎src/vs/workbench/contrib/debug/test/common/debugModel.test.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as assert from 'assert';
7-
import { ExceptionBreakpoint, FunctionBreakpoint } from 'vs/workbench/contrib/debug/common/debugModel';
7+
import { DeferredPromise } from 'vs/base/common/async';
8+
import { mockObject } from 'vs/base/test/common/mock';
9+
import { NullLogService } from 'vs/platform/log/common/log';
10+
import { DebugModel, ExceptionBreakpoint, FunctionBreakpoint, Thread } from 'vs/workbench/contrib/debug/common/debugModel';
11+
import { MockDebugStorage } from 'vs/workbench/contrib/debug/test/common/mockDebug';
812

913
suite('DebugModel', () => {
1014
suite('FunctionBreakpoint', () => {
@@ -15,6 +19,7 @@ suite('DebugModel', () => {
1519
assert.equal(parsed.id, fbp.getId());
1620
});
1721
});
22+
1823
suite('ExceptionBreakpoint', () => {
1924
test('Restored matches new', () => {
2025
const ebp = new ExceptionBreakpoint('id', 'label', true, true, 'condition', 'description', 'condition description', false);
@@ -24,4 +29,47 @@ suite('DebugModel', () => {
2429
assert.ok(ebp.matches(newEbp));
2530
});
2631
});
32+
33+
suite('DebugModel', () => {
34+
test('refreshTopOfCallstack resolves all returned promises when called multiple times', async () => {
35+
const topFrameDeferred = new DeferredPromise<void>();
36+
const wholeStackDeferred = new DeferredPromise<void>();
37+
const fakeThread = mockObject<Thread>()({
38+
session: { capabilities: { supportsDelayedStackTraceLoading: true } } as any,
39+
});
40+
fakeThread.fetchCallStack.callsFake((levels: number) => {
41+
return levels === 1 ? topFrameDeferred.p : wholeStackDeferred.p;
42+
});
43+
fakeThread.getId.returns(1);
44+
45+
const model = new DebugModel(new MockDebugStorage(), <any>{ isDirty: (e: any) => false }, undefined!, new NullLogService());
46+
47+
let top1Resolved = false;
48+
let whole1Resolved = false;
49+
let top2Resolved = false;
50+
let whole2Resolved = false;
51+
const result1 = model.refreshTopOfCallstack(fakeThread as any);
52+
result1.topCallStack.then(() => top1Resolved = true);
53+
result1.wholeCallStack.then(() => whole1Resolved = true);
54+
55+
const result2 = model.refreshTopOfCallstack(fakeThread as any);
56+
result2.topCallStack.then(() => top2Resolved = true);
57+
result2.wholeCallStack.then(() => whole2Resolved = true);
58+
59+
assert.ok(!top1Resolved);
60+
assert.ok(!whole1Resolved);
61+
assert.ok(!top2Resolved);
62+
assert.ok(!whole2Resolved);
63+
64+
await topFrameDeferred.complete();
65+
await result1.topCallStack;
66+
await result2.topCallStack;
67+
assert.ok(!whole1Resolved);
68+
assert.ok(!whole2Resolved);
69+
70+
await wholeStackDeferred.complete();
71+
await result1.wholeCallStack;
72+
await result2.wholeCallStack;
73+
});
74+
});
2775
});

0 commit comments

Comments
 (0)