Skip to content

Commit

Permalink
Better fix for previous candidate debug issue (#182930)
Browse files Browse the repository at this point in the history 
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 #181966
  • Loading branch information
@roblourens
roblourens authored May 19, 2023
1 parent 42dc9f0 commit aa79bd7
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 94 deletions.
155 changes: 81 additions & 74 deletions src/vs/workbench/contrib/debug/browser/debugSession.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -975,76 +975,81 @@ export class DebugSession implements IDebugSession {
}
}));

const statusQueue = new Queue<void>();
this.rawListeners.push(this.raw.onDidStop(async event => {
this.passFocusScheduler.cancel();
this.stoppedDetails.push(event.body);
await this.fetchThreads(event.body);
// If the focus for the current session is on a non-existent thread, clear the focus.
const focusedThread = this.debugService.getViewModel().focusedThread;
const focusedThreadDoesNotExist = focusedThread !== undefined && focusedThread.session === this && !this.threads.has(focusedThread.threadId);
if (focusedThreadDoesNotExist) {
this.debugService.focusStackFrame(undefined, undefined);
}
const thread = typeof event.body.threadId === 'number' ? this.getThread(event.body.threadId) : undefined;
if (thread) {
// Call fetch call stack twice, the first only return the top stack frame.
// Second retrieves the rest of the call stack. For performance reasons #25605
const promises = this.model.refreshTopOfCallstack(<Thread>thread);
const focus = async () => {
if (focusedThreadDoesNotExist || (!event.body.preserveFocusHint && thread.getCallStack().length)) {
const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame;
if (!focusedStackFrame || focusedStackFrame.thread.session === this) {
// Only take focus if nothing is focused, or if the focus is already on the current session
const preserveFocus = !this.configurationService.getValue<IDebugConfiguration>('debug').focusEditorOnBreak;
await this.debugService.focusStackFrame(undefined, thread, undefined, { preserveFocus });
}

if (thread.stoppedDetails) {
if (thread.stoppedDetails.reason === 'breakpoint' && this.configurationService.getValue<IDebugConfiguration>('debug').openDebug === 'openOnDebugBreak' && !this.suppressDebugView) {
await this.paneCompositeService.openPaneComposite(VIEWLET_ID, ViewContainerLocation.Sidebar);
statusQueue.queue(async () => {
this.passFocusScheduler.cancel();
this.stoppedDetails.push(event.body);
await this.fetchThreads(event.body);
// If the focus for the current session is on a non-existent thread, clear the focus.
const focusedThread = this.debugService.getViewModel().focusedThread;
const focusedThreadDoesNotExist = focusedThread !== undefined && focusedThread.session === this && !this.threads.has(focusedThread.threadId);
if (focusedThreadDoesNotExist) {
this.debugService.focusStackFrame(undefined, undefined);
}
const thread = typeof event.body.threadId === 'number' ? this.getThread(event.body.threadId) : undefined;
if (thread) {
// Call fetch call stack twice, the first only return the top stack frame.
// Second retrieves the rest of the call stack. For performance reasons #25605
const promises = this.model.refreshTopOfCallstack(<Thread>thread);
const focus = async () => {
if (focusedThreadDoesNotExist || (!event.body.preserveFocusHint && thread.getCallStack().length)) {
const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame;
if (!focusedStackFrame || focusedStackFrame.thread.session === this) {
// Only take focus if nothing is focused, or if the focus is already on the current session
const preserveFocus = !this.configurationService.getValue<IDebugConfiguration>('debug').focusEditorOnBreak;
await this.debugService.focusStackFrame(undefined, thread, undefined, { preserveFocus });
}

if (this.configurationService.getValue<IDebugConfiguration>('debug').focusWindowOnBreak && !this.workbenchEnvironmentService.extensionTestsLocationURI) {
await this.hostService.focus({ force: true /* Application may not be active */ });
if (thread.stoppedDetails) {
if (thread.stoppedDetails.reason === 'breakpoint' && this.configurationService.getValue<IDebugConfiguration>('debug').openDebug === 'openOnDebugBreak' && !this.suppressDebugView) {
await this.paneCompositeService.openPaneComposite(VIEWLET_ID, ViewContainerLocation.Sidebar);
}

if (this.configurationService.getValue<IDebugConfiguration>('debug').focusWindowOnBreak && !this.workbenchEnvironmentService.extensionTestsLocationURI) {
await this.hostService.focus({ force: true /* Application may not be active */ });
}
}
}
}
};

await promises.topCallStack;
focus();
await promises.wholeCallStack;
const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame;
if (!focusedStackFrame || !focusedStackFrame.source || focusedStackFrame.source.presentationHint === 'deemphasize' || focusedStackFrame.presentationHint === 'deemphasize') {
// The top stack frame can be deemphesized so try to focus again #68616
};

await promises.topCallStack;
focus();
await promises.wholeCallStack;
const focusedStackFrame = this.debugService.getViewModel().focusedStackFrame;
if (!focusedStackFrame || !focusedStackFrame.source || focusedStackFrame.source.presentationHint === 'deemphasize' || focusedStackFrame.presentationHint === 'deemphasize') {
// The top stack frame can be deemphesized so try to focus again #68616
focus();
}
}
}
this._onDidChangeState.fire();
this._onDidChangeState.fire();
});
}));

this.rawListeners.push(this.raw.onDidThread(event => {
if (event.body.reason === 'started') {
// debounce to reduce threadsRequest frequency and improve performance
if (!this.fetchThreadsScheduler) {
this.fetchThreadsScheduler = new RunOnceScheduler(() => {
this.fetchThreads();
}, 100);
this.rawListeners.push(this.fetchThreadsScheduler);
}
if (!this.fetchThreadsScheduler.isScheduled()) {
this.fetchThreadsScheduler.schedule();
}
} else if (event.body.reason === 'exited') {
this.model.clearThreads(this.getId(), true, event.body.threadId);
const viewModel = this.debugService.getViewModel();
const focusedThread = viewModel.focusedThread;
this.passFocusScheduler.cancel();
if (focusedThread && event.body.threadId === focusedThread.threadId) {
// De-focus the thread in case it was focused
this.debugService.focusStackFrame(undefined, undefined, viewModel.focusedSession, { explicit: false });
statusQueue.queue(async () => {
if (event.body.reason === 'started') {
// debounce to reduce threadsRequest frequency and improve performance
if (!this.fetchThreadsScheduler) {
this.fetchThreadsScheduler = new RunOnceScheduler(() => {
this.fetchThreads();
}, 100);
this.rawListeners.push(this.fetchThreadsScheduler);
}
if (!this.fetchThreadsScheduler.isScheduled()) {
this.fetchThreadsScheduler.schedule();
}
} else if (event.body.reason === 'exited') {
this.model.clearThreads(this.getId(), true, event.body.threadId);
const viewModel = this.debugService.getViewModel();
const focusedThread = viewModel.focusedThread;
this.passFocusScheduler.cancel();
if (focusedThread && event.body.threadId === focusedThread.threadId) {
// De-focus the thread in case it was focused
this.debugService.focusStackFrame(undefined, undefined, viewModel.focusedSession, { explicit: false });
}
}
}
});
}));

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

this.rawListeners.push(this.raw.onDidContinued(event => {
const threadId = event.body.allThreadsContinued !== false ? undefined : event.body.threadId;
if (typeof threadId === 'number') {
this.stoppedDetails = this.stoppedDetails.filter(sd => sd.threadId !== threadId);
const tokens = this.cancellationMap.get(threadId);
this.cancellationMap.delete(threadId);
tokens?.forEach(t => t.cancel());
} else {
this.stoppedDetails = [];
this.cancelAllRequests();
}
this.lastContinuedThreadId = threadId;
// We need to pass focus to other sessions / threads with a timeout in case a quick stop event occurs #130321
this.passFocusScheduler.schedule();
this.model.clearThreads(this.getId(), false, threadId);
this._onDidChangeState.fire();
statusQueue.queue(async () => {
const threadId = event.body.allThreadsContinued !== false ? undefined : event.body.threadId;
if (typeof threadId === 'number') {
this.stoppedDetails = this.stoppedDetails.filter(sd => sd.threadId !== threadId);
const tokens = this.cancellationMap.get(threadId);
this.cancellationMap.delete(threadId);
tokens?.forEach(t => t.cancel());
} else {
this.stoppedDetails = [];
this.cancelAllRequests();
}
this.lastContinuedThreadId = threadId;
// We need to pass focus to other sessions / threads with a timeout in case a quick stop event occurs #130321
this.passFocusScheduler.schedule();
this.model.clearThreads(this.getId(), false, threadId);
this._onDidChangeState.fire();
});
}));

const outputQueue = new Queue<void>();
Expand Down
49 changes: 30 additions & 19 deletions src/vs/workbench/contrib/debug/common/debugModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

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

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

clearThreads(id: string, removeThreads: boolean, reference: number | undefined = undefined): void {
const session = this.sessions.find(p => p.getId() === id);
this.schedulers.forEach(scheduler => scheduler.dispose());
this.schedulers.forEach(entry => {
entry.scheduler.dispose();
entry.completeDeferred.complete();
});
this.schedulers.clear();

if (session) {
Expand Down Expand Up @@ -1314,24 +1317,32 @@ export class DebugModel implements IDebugModel {
const wholeCallStack = new Promise<void>((c, e) => {
topCallStack = thread.fetchCallStack(1).then(() => {
if (!this.schedulers.has(thread.getId())) {
this.schedulers.set(thread.getId(), new RunOnceScheduler(() => {
thread.fetchCallStack(19).then(() => {
const stale = thread.getStaleCallStack();
const current = thread.getCallStack();
let bottomOfCallStackChanged = stale.length !== current.length;
for (let i = 1; i < stale.length && !bottomOfCallStackChanged; i++) {
bottomOfCallStackChanged = !stale[i].equals(current[i]);
}

if (bottomOfCallStackChanged) {
this._onDidChangeCallStack.fire();
}
c();
});
}, 420));
const deferred = new DeferredPromise<void>();
this.schedulers.set(thread.getId(), {
completeDeferred: deferred,
scheduler: new RunOnceScheduler(() => {
thread.fetchCallStack(19).then(() => {
const stale = thread.getStaleCallStack();
const current = thread.getCallStack();
let bottomOfCallStackChanged = stale.length !== current.length;
for (let i = 1; i < stale.length && !bottomOfCallStackChanged; i++) {
bottomOfCallStackChanged = !stale[i].equals(current[i]);
}

if (bottomOfCallStackChanged) {
this._onDidChangeCallStack.fire();
}
}).finally(() => {
deferred.complete();
this.schedulers.delete(thread.getId());
});
}, 420)
});
}

this.schedulers.get(thread.getId())!.schedule();
const entry = this.schedulers.get(thread.getId())!;
entry.scheduler.schedule();
entry.completeDeferred.p.then(c, e);
this._onDidChangeCallStack.fire();
});
});
Expand Down
50 changes: 49 additions & 1 deletion src/vs/workbench/contrib/debug/test/common/debugModel.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { ExceptionBreakpoint, FunctionBreakpoint } from 'vs/workbench/contrib/debug/common/debugModel';
import { DeferredPromise } from 'vs/base/common/async';
import { mockObject } from 'vs/base/test/common/mock';
import { NullLogService } from 'vs/platform/log/common/log';
import { DebugModel, ExceptionBreakpoint, FunctionBreakpoint, Thread } from 'vs/workbench/contrib/debug/common/debugModel';
import { MockDebugStorage } from 'vs/workbench/contrib/debug/test/common/mockDebug';

suite('DebugModel', () => {
suite('FunctionBreakpoint', () => {
Expand All @@ -15,6 +19,7 @@ suite('DebugModel', () => {
assert.equal(parsed.id, fbp.getId());
});
});

suite('ExceptionBreakpoint', () => {
test('Restored matches new', () => {
const ebp = new ExceptionBreakpoint('id', 'label', true, true, 'condition', 'description', 'condition description', false);
Expand All @@ -24,4 +29,47 @@ suite('DebugModel', () => {
assert.ok(ebp.matches(newEbp));
});
});

suite('DebugModel', () => {
test('refreshTopOfCallstack resolves all returned promises when called multiple times', async () => {
const topFrameDeferred = new DeferredPromise<void>();
const wholeStackDeferred = new DeferredPromise<void>();
const fakeThread = mockObject<Thread>()({
session: { capabilities: { supportsDelayedStackTraceLoading: true } } as any,
});
fakeThread.fetchCallStack.callsFake((levels: number) => {
return levels === 1 ? topFrameDeferred.p : wholeStackDeferred.p;
});
fakeThread.getId.returns(1);

const model = new DebugModel(new MockDebugStorage(), <any>{ isDirty: (e: any) => false }, undefined!, new NullLogService());

let top1Resolved = false;
let whole1Resolved = false;
let top2Resolved = false;
let whole2Resolved = false;
const result1 = model.refreshTopOfCallstack(fakeThread as any);
result1.topCallStack.then(() => top1Resolved = true);
result1.wholeCallStack.then(() => whole1Resolved = true);

const result2 = model.refreshTopOfCallstack(fakeThread as any);
result2.topCallStack.then(() => top2Resolved = true);
result2.wholeCallStack.then(() => whole2Resolved = true);

assert.ok(!top1Resolved);
assert.ok(!whole1Resolved);
assert.ok(!top2Resolved);
assert.ok(!whole2Resolved);

await topFrameDeferred.complete();
await result1.topCallStack;
await result2.topCallStack;
assert.ok(!whole1Resolved);
assert.ok(!whole2Resolved);

await wholeStackDeferred.complete();
await result1.wholeCallStack;
await result2.wholeCallStack;
});
});
});

0 comments on commit aa79bd7

Please sign in to comment.