Skip to content

Commit aa1b0b2

Browse files
joyceerhljoyceerhl
authored
Ensure debug adapter request/response sequence numbers tally (#14336)
1 parent 904084f commit aa1b0b2

File tree

5 files changed

+107
-62
lines changed

5 files changed

+107
-62
lines changed

‎src/client/datascience/debugLocationTracker.ts‎

Lines changed: 59 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
'use strict';
4+
import { injectable } from 'inversify';
45
import { DebugAdapterTracker, Event, EventEmitter } from 'vscode';
56
import { DebugProtocol } from 'vscode-debugprotocol';
67

78
import { IDebugLocation } from './types';
89

910
// When a python debugging session is active keep track of the current debug location
11+
@injectable()
1012
export class DebugLocationTracker implements DebugAdapterTracker {
11-
private waitingForStackTrace: boolean = false;
13+
protected topMostFrameId = 0;
14+
protected sequenceNumbersOfRequestsPendingResponses = new Set<number>();
15+
private waitingForStackTrace = false;
1216
private _debugLocation: IDebugLocation | undefined;
1317
private debugLocationUpdatedEvent: EventEmitter<void> = new EventEmitter<void>();
1418
private sessionEndedEmitter: EventEmitter<DebugLocationTracker> = new EventEmitter<DebugLocationTracker>();
1519

16-
constructor(private _sessionId: string) {
20+
constructor(private _sessionId: string | undefined) {
1721
this.DebugLocation = undefined;
1822
}
1923

@@ -33,8 +37,22 @@ export class DebugLocationTracker implements DebugAdapterTracker {
3337
return this._debugLocation;
3438
}
3539

36-
// tslint:disable-next-line:no-any
37-
public onDidSendMessage(message: DebugProtocol.ProtocolMessage) {
40+
public onDidSendMessage(message: DebugProtocol.Response) {
41+
if (this.isResponseForRequestToFetchAllFrames(message)) {
42+
// This should be the top frame. We need to use this to compute the value of a variable
43+
const topMostFrame = message.body.stackFrames[0];
44+
this.topMostFrameId = topMostFrame?.id;
45+
this.sequenceNumbersOfRequestsPendingResponses.delete(message.request_seq);
46+
// If we are waiting for a stack trace, check our messages for one
47+
if (this.waitingForStackTrace) {
48+
this.DebugLocation = {
49+
lineNumber: topMostFrame?.line,
50+
fileName: this.normalizeFilePath(topMostFrame?.source?.path),
51+
column: topMostFrame.column
52+
};
53+
this.waitingForStackTrace = false;
54+
}
55+
}
3856
if (this.isStopEvent(message)) {
3957
// Some type of stop, wait to see our next stack trace to find our location
4058
this.waitingForStackTrace = true;
@@ -45,21 +63,23 @@ export class DebugLocationTracker implements DebugAdapterTracker {
4563
this.DebugLocation = undefined;
4664
this.waitingForStackTrace = false;
4765
}
48-
49-
if (this.waitingForStackTrace) {
50-
// If we are waiting for a stack track, check our messages for one
51-
const debugLoc = this.getStackTrace(message);
52-
if (debugLoc) {
53-
this.DebugLocation = debugLoc;
54-
this.waitingForStackTrace = false;
55-
}
56-
}
5766
}
5867

5968
public onWillStopSession() {
6069
this.sessionEndedEmitter.fire(this);
6170
}
6271

72+
public onWillReceiveMessage(message: DebugProtocol.Request) {
73+
if (this.isRequestToFetchAllFrames(message)) {
74+
// VSCode sometimes sends multiple stackTrace requests. The true topmost frame is determined
75+
// based on the response to a stackTrace request where the startFrame is 0 or undefined (i.e.
76+
// this request retrieves all frames). Here, remember the sequence number of the outgoing
77+
// request whose startFrame === 0 or undefined, and update this.topMostFrameId only when we
78+
// receive the response with a matching sequence number.
79+
this.sequenceNumbersOfRequestsPendingResponses.add(message.seq);
80+
}
81+
}
82+
6383
// Set our new location and fire our debug event
6484
private set DebugLocation(newLocation: IDebugLocation | undefined) {
6585
const oldLocation = this._debugLocation;
@@ -70,7 +90,15 @@ export class DebugLocationTracker implements DebugAdapterTracker {
7090
}
7191
}
7292

73-
// tslint:disable-next-line:no-any
93+
private normalizeFilePath(path: string): string {
94+
// Make the path match the os. Debugger seems to return
95+
// invalid path chars on linux/darwin
96+
if (process.platform !== 'win32') {
97+
return path.replace(/\\/g, '/');
98+
}
99+
return path;
100+
}
101+
74102
private isStopEvent(message: DebugProtocol.ProtocolMessage) {
75103
if (message.type === 'event') {
76104
const eventMessage = message as DebugProtocol.Event;
@@ -82,34 +110,6 @@ export class DebugLocationTracker implements DebugAdapterTracker {
82110
return false;
83111
}
84112

85-
// tslint:disable-next-line:no-any
86-
private getStackTrace(message: DebugProtocol.ProtocolMessage): IDebugLocation | undefined {
87-
if (message.type === 'response') {
88-
const responseMessage = message as DebugProtocol.Response;
89-
if (responseMessage.command === 'stackTrace') {
90-
const messageBody = responseMessage.body;
91-
if (messageBody.stackFrames.length > 0) {
92-
const lineNumber = messageBody.stackFrames[0].line;
93-
const fileName = this.normalizeFilePath(messageBody.stackFrames[0].source.path);
94-
const column = messageBody.stackFrames[0].column;
95-
return { lineNumber, fileName, column };
96-
}
97-
}
98-
}
99-
100-
return undefined;
101-
}
102-
103-
private normalizeFilePath(path: string): string {
104-
// Make the path match the os. Debugger seems to return
105-
// invalid path chars on linux/darwin
106-
if (process.platform !== 'win32') {
107-
return path.replace(/\\/g, '/');
108-
}
109-
return path;
110-
}
111-
112-
// tslint:disable-next-line:no-any
113113
private isContinueEvent(message: DebugProtocol.ProtocolMessage): boolean {
114114
if (message.type === 'event') {
115115
const eventMessage = message as DebugProtocol.Event;
@@ -125,4 +125,21 @@ export class DebugLocationTracker implements DebugAdapterTracker {
125125

126126
return false;
127127
}
128+
129+
private isResponseForRequestToFetchAllFrames(message: DebugProtocol.Response) {
130+
return (
131+
message.type === 'response' &&
132+
message.command === 'stackTrace' &&
133+
message.body.stackFrames[0] &&
134+
this.sequenceNumbersOfRequestsPendingResponses.has(message.request_seq)
135+
);
136+
}
137+
138+
private isRequestToFetchAllFrames(message: DebugProtocol.Request) {
139+
return (
140+
message.type === 'request' &&
141+
message.command === 'stackTrace' &&
142+
(message.arguments.startFrame === 0 || message.arguments.startFrame === undefined)
143+
);
144+
}
128145
}

‎src/client/datascience/debugLocationTrackerFactory.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ export class DebugLocationTrackerFactory implements IDebugLocationTracker, Debug
5050
}
5151

5252
private onSessionEnd(locationTracker: DebugLocationTracker) {
53-
this.activeTrackers.delete(locationTracker.sessionId);
53+
if (locationTracker.sessionId) {
54+
this.activeTrackers.delete(locationTracker.sessionId);
55+
}
5456
}
5557

5658
private onLocationUpdated() {

‎src/client/datascience/jupyter/debuggerVariables.ts‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { traceError } from '../../common/logger';
1010
import { IConfigurationService, Resource } from '../../common/types';
1111
import { sendTelemetryEvent } from '../../telemetry';
1212
import { DataFrameLoading, Identifiers, Telemetry } from '../constants';
13+
import { DebugLocationTracker } from '../debugLocationTracker';
1314
import {
1415
IConditionalJupyterVariables,
1516
IJupyterDebugService,
@@ -23,17 +24,19 @@ const DataViewableTypes: Set<string> = new Set<string>(['DataFrame', 'list', 'di
2324
const KnownExcludedVariables = new Set<string>(['In', 'Out', 'exit', 'quit']);
2425

2526
@injectable()
26-
export class DebuggerVariables implements IConditionalJupyterVariables, DebugAdapterTracker {
27+
export class DebuggerVariables extends DebugLocationTracker
28+
implements IConditionalJupyterVariables, DebugAdapterTracker {
2729
private refreshEventEmitter = new EventEmitter<void>();
2830
private lastKnownVariables: IJupyterVariable[] = [];
29-
private topMostFrameId = 0;
3031
private importedIntoKernel = new Set<string>();
3132
private watchedNotebooks = new Map<string, Disposable[]>();
3233
private debuggingStarted = false;
3334
constructor(
3435
@inject(IJupyterDebugService) @named(Identifiers.MULTIPLEXING_DEBUGSERVICE) private debugService: IDebugService,
3536
@inject(IConfigurationService) private configService: IConfigurationService
36-
) {}
37+
) {
38+
super(undefined);
39+
}
3740

3841
public get refreshRequired(): Event<void> {
3942
return this.refreshEventEmitter.event;
@@ -110,10 +113,12 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda
110113
);
111114

112115
// Results should be the updated variable.
113-
return {
114-
...targetVariable,
115-
...JSON.parse(results.result.slice(1, -1))
116-
};
116+
return results
117+
? {
118+
...targetVariable,
119+
...JSON.parse(results.result.slice(1, -1))
120+
}
121+
: targetVariable;
117122
}
118123

119124
public async getDataFrameRows(
@@ -166,6 +171,7 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda
166171

167172
// tslint:disable-next-line: no-any
168173
public onDidSendMessage(message: any) {
174+
super.onDidSendMessage(message);
169175
// When the initialize response comes back, indicate we have started.
170176
if (message.type === 'response' && message.command === 'initialize') {
171177
this.debuggingStarted = true;
@@ -174,9 +180,6 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda
174180
// tslint:disable-next-line: no-suspicious-comment
175181
// TODO: Figure out what resource to use
176182
this.updateVariables(undefined, message as DebugProtocol.VariablesResponse);
177-
} else if (message.type === 'response' && message.command === 'stackTrace') {
178-
// This should be the top frame. We need to use this to compute the value of a variable
179-
this.updateStackFrame(message as DebugProtocol.StackTraceResponse);
180183
} else if (message.type === 'event' && message.event === 'terminated') {
181184
// When the debugger exits, make sure the variables are cleared
182185
this.lastKnownVariables = [];
@@ -240,12 +243,6 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda
240243
}
241244
}
242245

243-
private updateStackFrame(stackResponse: DebugProtocol.StackTraceResponse) {
244-
if (stackResponse.body.stackFrames[0]) {
245-
this.topMostFrameId = stackResponse.body.stackFrames[0].id;
246-
}
247-
}
248-
249246
private async getFullVariable(variable: IJupyterVariable, notebook: INotebook): Promise<IJupyterVariable> {
250247
// See if we imported or not into the kernel our special function
251248
await this.importDataFrameScripts(notebook);

‎src/client/datascience/jupyterDebugService.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,18 @@ export class JupyterDebugService implements IJupyterDebugService, IDisposable {
260260
}
261261

262262
private sendToTrackers(args: any) {
263-
this.debugAdapterTrackers.forEach((d) => d.onDidSendMessage!(args));
263+
switch (args.type) {
264+
case 'request':
265+
this.debugAdapterTrackers.forEach((d) => {
266+
if (d.onWillReceiveMessage) {
267+
d.onWillReceiveMessage(args);
268+
}
269+
});
270+
break;
271+
default:
272+
this.debugAdapterTrackers.forEach((d) => d.onDidSendMessage!(args));
273+
break;
274+
}
264275
}
265276

266277
private sendCustomRequest(command: string, args?: any): Promise<any> {

‎src/test/datascience/debugLocationTracker.unit.test.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
'use strict';
44
//tslint:disable:max-func-body-length match-default-export-name no-any no-multiline-string no-trailing-whitespace
55
import { expect } from 'chai';
6+
import { DebugProtocol } from 'vscode-debugprotocol';
67

78
import { DebugLocationTracker } from '../../client/datascience/debugLocationTracker';
89
import { IDebugLocation } from '../../client/datascience/types';
@@ -21,6 +22,7 @@ suite('Debug Location Tracker', () => {
2122

2223
expect(debugTracker.debugLocation).to.be.equal(undefined, 'After stop location is empty');
2324

25+
debugTracker.onWillReceiveMessage(makeStackTraceRequest());
2426
debugTracker.onDidSendMessage(makeStackTraceMessage());
2527

2628
const testLocation: IDebugLocation = { lineNumber: 1, column: 1, fileName: 'testpath' };
@@ -40,12 +42,28 @@ function makeContinueMessage(): any {
4042
return { type: 'event', event: 'continue' };
4143
}
4244

43-
function makeStackTraceMessage(): any {
45+
function makeStackTraceMessage(): DebugProtocol.Response {
4446
return {
4547
type: 'response',
4648
command: 'stackTrace',
49+
request_seq: 42,
50+
success: true,
51+
seq: 43,
4752
body: {
48-
stackFrames: [{ line: 1, column: 1, source: { path: 'testpath' } }]
53+
stackFrames: [{ id: 9000, line: 1, column: 1, source: { path: 'testpath' } }]
54+
}
55+
};
56+
}
57+
58+
function makeStackTraceRequest(): DebugProtocol.Request {
59+
return {
60+
type: 'request',
61+
command: 'stackTrace',
62+
seq: 42,
63+
arguments: {
64+
levels: 1,
65+
startFrame: 0,
66+
threadId: 1
4967
}
5068
};
5169
}

0 commit comments

Comments
 (0)