Skip to content

Commit 1401d19

Browse files
felixfbeckerfelixfbecker
committed
Refactor new connection handling
Removes PhpDebugSession.prototype._mainConnection. The first connection is not treated differently than following connections anymore and breakpoints are not stored anymore. Instead, on each new connection an InitializedEvent is triggered. Connections that have not yet been initialized with breakpoints are stored in a Set, which is checked in both setBreakpointRequest and setExceptionBreakpointRequest. This fixes a race condition between the two which could result in a run command before all breakpoints were set. The two Sets are a workaround until ConfigurationDoneRequest ships in VS Code.
1 parent d7e4761 commit 1401d19

File tree

1 file changed

+82
-111
lines changed

1 file changed

+82
-111
lines changed

‎src/phpDebug.ts‎

Lines changed: 82 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -66,28 +66,38 @@ class PhpDebugSession extends vscode.DebugSession {
6666

6767
/** The arguments that were given to launchRequest */
6868
private _args: LaunchRequestArguments;
69+
6970
/** The TCP server that listens for XDebug connections */
7071
private _server: net.Server;
71-
/** All XDebug Connections. XDebug makes a new connection for each request to the webserver. */
72+
73+
/**
74+
* A map from VS Code thread IDs to XDebug Connections.
75+
* XDebug makes a new connection for each request to the webserver, we present hese as threads to VS Code.
76+
* The threadId key is equal to the id attribute of the connection.
77+
*/
7278
private _connections = new Map<number, xdebug.Connection>();
73-
/** The first connection we receive */
74-
private _mainConnection: xdebug.Connection = null;
75-
/** Gets set to true after _runOrStopOnEntry is called the first time and means all exceptions etc. are set */
76-
private _initialized = false;
77-
/** A map of file URIs to lines: breakpoints received from VS Code */
78-
private _breakpoints = new Map<string, number[]>();
79-
/** Gets set after a setExceptionBreakpointsRequest */
80-
private _breakOnExceptions: boolean;
79+
80+
/** A set of connecitons which still need to be initialized with exception breakpoints before _runOrStopOnEntry can be called. */
81+
private _connectionsAwaitingExceptionBreakpoints = new Set<xdebug.Connection>();
82+
83+
/** A set of connecitons which still need to be initialized with exception breakpoints before _runOrStopOnEntry can be called. */
84+
private _connectionsAwaitingBreakpoints = new Set<xdebug.Connection>();
85+
8186
/** A counter for unique stackframe IDs */
8287
private _stackFrameIdCounter = 1;
88+
8389
/** A map from unique stackframe IDs (even across connections) to XDebug stackframes */
8490
private _stackFrames = new Map<number, xdebug.StackFrame>();
91+
8592
/** A counter for unique context, property and eval result properties (as these are all requested by a VariableRequest from VS Code) */
8693
private _variableIdCounter = 1;
94+
8795
/** A map from unique VS Code variable IDs to an XDebug contexts */
8896
private _contexts = new Map<number, xdebug.Context>();
97+
8998
/** A map from unique VS Code variable IDs to a XDebug properties */
9099
private _properties = new Map<number, xdebug.Property>();
100+
91101
/** A map from unique VS Code variable IDs to XDebug eval result properties, because property children returned from eval commands are always inlined */
92102
private _evalResultProperties = new Map<number, xdebug.EvalResultProperty>();
93103

@@ -115,55 +125,22 @@ class PhpDebugSession extends vscode.DebugSession {
115125
// new XDebug connection
116126
const connection = new xdebug.Connection(socket);
117127
this._connections.set(connection.id, connection);
118-
if (this._initialized) {
119-
// this is a new connection, for example triggered by a seperate, parallel request to the webserver.
120-
connection.waitForInitPacket()
121-
// tell VS Code that this is a new thread
122-
.then(() => {
123-
this.sendEvent(new vscode.ThreadEvent('started', connection.id));
124-
})
125-
// set max_depth to 1 since VS Code requests nested structures individually anyway
126-
.then(() => connection.sendFeatureSetCommand('max_depth', '1'))
127-
// raise default of 32
128-
.then(() => connection.sendFeatureSetCommand('max_children', '9999'))
129-
// restore all breakpoints for the new connection
130-
.then(() => Promise.all(Array.from(this._breakpoints).map(([fileUri, lines]) =>
131-
Promise.all(lines.map(line =>
132-
connection.sendBreakpointSetCommand({type: 'line', fileUri, line})
133-
))
134-
)))
135-
// restore exception breakpoint settings for the new connection
136-
.then(() => {
137-
if (this._breakOnExceptions) {
138-
return connection.sendBreakpointSetCommand({type: 'exception', exception: '*'});
139-
}
140-
})
141-
// run the script or stop on entry
142-
.then(() => this._runOrStopOnEntry(connection))
143-
.catch(error => {
144-
console.error('error: ', error);
145-
});
146-
} else {
147-
// this is the first connection
148-
this._mainConnection = connection;
149-
connection.waitForInitPacket()
150-
// set max_depth to 1 since VS Code requests nested structures individually anyway
151-
.then(initPacket => {
152-
return connection.sendFeatureSetCommand('max_depth', '1');
153-
})
154-
// raise default of 32
155-
.then(response => {
156-
return connection.sendFeatureSetCommand('max_children', '9999')
157-
})
158-
.then(response => {
159-
// tell VS Code we are ready to accept breakpoints
160-
// once VS Code has set all breakpoints setExceptionBreakpointsRequest will automatically call _runOrStopOnEntry with the mainConnection.
161-
this.sendEvent(new vscode.InitializedEvent());
162-
})
163-
.catch(error => {
164-
console.error('error: ', error);
165-
});
166-
}
128+
this._connectionsAwaitingBreakpoints.add(connection);
129+
this._connectionsAwaitingExceptionBreakpoints.add(connection);
130+
connection.waitForInitPacket()
131+
.then(() => {
132+
this.sendEvent(new vscode.ThreadEvent('started', connection.id));
133+
})
134+
// set max_depth to 1 since VS Code requests nested structures individually anyway
135+
.then(initPacket => connection.sendFeatureSetCommand('max_depth', '1'))
136+
// raise default of 32
137+
.then(response => connection.sendFeatureSetCommand('max_children', '9999'))
138+
// request breakpoints from VS Code
139+
// once VS Code has set all breakpoints (eg breakpointsSet and exceptionBreakpointsSet are true) _runOrStopOnEntry will be called
140+
.then(response => this.sendEvent(new vscode.InitializedEvent()))
141+
.catch(error => {
142+
console.error('error: ', error);
143+
});
167144
});
168145
server.listen(args.port);
169146
this.sendResponse(response);
@@ -172,7 +149,6 @@ class PhpDebugSession extends vscode.DebugSession {
172149
/** is called after all breakpoints etc. are initialized and either runs the script or notifies VS Code that we stopped on entry, depending on launch settings */
173150
private _runOrStopOnEntry(connection: xdebug.Connection): void {
174151
// either tell VS Code we stopped on entry or run the script
175-
this._initialized = true;
176152
if (this._args.stopOnEntry) {
177153
this.sendEvent(new vscode.StoppedEvent('entry', connection.id));
178154
} else {
@@ -186,13 +162,9 @@ class PhpDebugSession extends vscode.DebugSession {
186162
if (response.status === 'stopping') {
187163
connection.sendStopCommand().then(response => this._checkStatus(response));
188164
} else if (response.status === 'stopped') {
189-
connection.close().then(() => {
190-
this._connections.delete(connection.id);
191-
if (this._mainConnection === connection) {
192-
this._mainConnection = null;
193-
}
194-
this.sendEvent(new vscode.ThreadEvent('exited', connection.id));
195-
});
165+
this._connections.delete(connection.id);
166+
this.sendEvent(new vscode.ThreadEvent('exited', connection.id));
167+
connection.close();
196168
} else if (response.status === 'break') {
197169
// StoppedEvent reason can be 'step', 'breakpoint', 'exception' or 'pause'
198170
let stoppedEventReason: string;
@@ -292,7 +264,7 @@ class PhpDebugSession extends vscode.DebugSession {
292264
breakpointsSetPromise = Promise.resolve();
293265
} else {
294266
breakpoints = [];
295-
breakpointsSetPromise = Promise.all(connections.map(connection =>
267+
breakpointsSetPromise = Promise.all(connections.map((connection, connectionIndex) =>
296268
// clear breakpoints for this file
297269
connection.sendBreakpointListCommand()
298270
.then(response => Promise.all(
@@ -304,25 +276,30 @@ class PhpDebugSession extends vscode.DebugSession {
304276
.then(() => Promise.all(args.lines.map(line =>
305277
connection.sendBreakpointSetCommand({type: 'line', fileUri, line})
306278
.then(xdebugResponse => {
307-
// only capture each breakpoint once (for the main connection)
308-
if (connection === this._mainConnection) {
279+
// remember that the breakpoints for this connection have been set
280+
this._connectionsAwaitingBreakpoints.delete(connection);
281+
// if this connection has also already received exception breakpoints, run it
282+
if (!this._connectionsAwaitingExceptionBreakpoints.has(connection)) {
283+
this._runOrStopOnEntry(connection);
284+
}
285+
// only capture each breakpoint once
286+
if (connectionIndex === 0) {
309287
breakpoints.push(new vscode.Breakpoint(true, line));
310288
}
311289
})
312290
.catch(error => {
313-
// only capture each breakpoint once (for the main connection)
314-
if (connection === this._mainConnection) {
291+
// only capture each breakpoint once
292+
if (connectionIndex === 0) {
315293
console.error('breakpoint could not be set: ', error);
316294
breakpoints.push(new vscode.Breakpoint(false, line));
317295
}
318296
})
319297
)))
320-
))
298+
));
321299
}
322300
breakpointsSetPromise
323301
.then(() => {
324302
response.body = {breakpoints};
325-
this._breakpoints.set(fileUri, args.lines);
326303
this.sendResponse(response);
327304
})
328305
.catch(error => {
@@ -337,38 +314,35 @@ class PhpDebugSession extends vscode.DebugSession {
337314
if (args.filters.indexOf('all') !== -1) {
338315
this.sendEvent(new vscode.OutputEvent('breaking on caught exceptions is not supported by XDebug', 'stderr'));
339316
}
340-
Promise.resolve()
341-
.then<any>(() => {
342-
// does the new setting differ from the current setting?
343-
if (breakOnExceptions !== !!this._breakOnExceptions) {
344-
const connections = Array.from(this._connections.values());
317+
const connections = Array.from(this._connections.values());
318+
// remove all exception breakpoints
319+
Promise.all(connections.map(connection =>
320+
// remove all exception breakpoints
321+
connection.sendBreakpointListCommand()
322+
.then(response => Promise.all(
323+
response.breakpoints
324+
.filter(breakpoint => breakpoint.type === 'exception')
325+
.map(breakpoint => breakpoint.remove())
326+
))
327+
.then(() => {
328+
// if enabled, set exception breakpoint for all exceptions
345329
if (breakOnExceptions) {
346-
// set an exception breakpoint for all exceptions
347-
return Promise.all(connections.map(connection => connection.sendBreakpointSetCommand({type: 'exception', exception: '*'})));
348-
} else {
349-
// remove all exception breakpoints
350-
return Promise.all(connections.map(connection =>
351-
connection.sendBreakpointListCommand()
352-
.then(response => Promise.all(
353-
response.breakpoints
354-
.filter(breakpoint => breakpoint.type === 'exception')
355-
.map(breakpoint => breakpoint.remove())
356-
))
357-
));
330+
return connection.sendBreakpointSetCommand({type: 'exception', exception: '*'});
358331
}
359-
}
360-
})
361-
.then(() => {
362-
this._breakOnExceptions = breakOnExceptions;
363-
this.sendResponse(response);
364-
// if this is the first time this is called and the main connection is not yet running, trigger a run because now everything is set up
365-
if (!this._initialized) {
366-
this._runOrStopOnEntry(this._mainConnection);
367-
}
368-
})
369-
.catch(error => {
370-
this.sendErrorResponse(response, error.code, error.message);
371-
})
332+
})
333+
.then(() => {
334+
// remember that the exception breakpoints for this connection have been set
335+
this._connectionsAwaitingExceptionBreakpoints.delete(connection);
336+
// if this connection has also already received line breakpoints, run it
337+
if (!this._connectionsAwaitingBreakpoints.has(connection)) {
338+
this._runOrStopOnEntry(connection);
339+
}
340+
})
341+
)).then(() => {
342+
this.sendResponse(response);
343+
}).catch(error => {
344+
this.sendErrorResponse(response, error.code, error.message);
345+
});
372346
}
373347

374348
/** Executed after a successfull launch or attach request and after a ThreadEvent */
@@ -486,31 +460,31 @@ class PhpDebugSession extends vscode.DebugSession {
486460
}
487461

488462
protected continueRequest(response: VSCodeDebugProtocol.ContinueResponse, args: VSCodeDebugProtocol.ContinueArguments): void {
489-
const connection = this._connections.get(args.threadId) || this._mainConnection;
463+
const connection = this._connections.get(args.threadId);
490464
connection.sendRunCommand()
491465
.then(response => this._checkStatus(response))
492466
.catch(error => this.sendErrorResponse(response, error.code, error.message));
493467
this.sendResponse(response);
494468
}
495469

496470
protected nextRequest(response: VSCodeDebugProtocol.NextResponse, args: VSCodeDebugProtocol.NextArguments): void {
497-
const connection = this._connections.get(args.threadId) || this._mainConnection;
471+
const connection = this._connections.get(args.threadId);
498472
connection.sendStepOverCommand()
499473
.then(response => this._checkStatus(response))
500474
.catch(error => this.sendErrorResponse(response, error.code, error.message));
501475
this.sendResponse(response);
502476
}
503477

504478
protected stepInRequest(response: VSCodeDebugProtocol.StepInResponse, args: VSCodeDebugProtocol.StepInArguments) : void {
505-
const connection = this._connections.get(args.threadId) || this._mainConnection;
479+
const connection = this._connections.get(args.threadId);
506480
connection.sendStepIntoCommand()
507481
.then(response => this._checkStatus(response))
508482
.catch(error => this.sendErrorResponse(response, error.code, error.message));
509483
this.sendResponse(response);
510484
}
511485

512486
protected stepOutRequest(response: VSCodeDebugProtocol.StepOutResponse, args: VSCodeDebugProtocol.StepOutArguments) : void {
513-
const connection = this._connections.get(args.threadId) || this._mainConnection;
487+
const connection = this._connections.get(args.threadId);
514488
connection.sendStepOutCommand()
515489
.then(response => this._checkStatus(response))
516490
.catch(error => this.sendErrorResponse(response, error.code, error.message));
@@ -527,9 +501,6 @@ class PhpDebugSession extends vscode.DebugSession {
527501
.then(response => connection.close())
528502
.then(() => {
529503
this._connections.delete(id);
530-
if (this._mainConnection === connection) {
531-
this._mainConnection = null;
532-
}
533504
})
534505
.catch(() => {})
535506
)).then(() => {
@@ -543,7 +514,7 @@ class PhpDebugSession extends vscode.DebugSession {
543514
}
544515

545516
protected evaluateRequest(response: VSCodeDebugProtocol.EvaluateResponse, args: VSCodeDebugProtocol.EvaluateArguments): void {
546-
const connection = this._stackFrames.has(args.frameId) ? this._stackFrames.get(args.frameId).connection : this._mainConnection;
517+
const connection = this._stackFrames.get(args.frameId).connection;
547518
connection.sendEvalCommand(args.expression)
548519
.then(xdebugResponse => {
549520
if (xdebugResponse.result) {

0 commit comments

Comments
 (0)