Skip to content

Commit 6c6acac

Browse files
DanTupDanTup
committed
Switch from pseudo terminals to real terminal processes
Debug adapter sends a custom event to the editor allowing it to spawn the process locally, and watches for the --write-service-info file to connect to the VM. Fixes #2230.
1 parent 8376f4f commit 6c6acac

5 files changed

Lines changed: 141 additions & 161 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1559,7 +1559,7 @@
15591559
"terminal"
15601560
],
15611561
"default": "debugConsole",
1562-
"description": "Whether to run applications in the debug console (which does not support reading user input) or a pseudo terminal (which does)."
1562+
"description": "Whether to run applications in the debug console (which does not support reading user input) or a VS Code terminal (which does)."
15631563
},
15641564
"enableAsserts": {
15651565
"type": "boolean",

‎src/debug/dart_debug_impl.ts‎

Lines changed: 115 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { observatoryListeningBannerPattern, pleaseReportBug } from "../shared/co
1111
import { LogCategory, LogSeverity } from "../shared/enums";
1212
import { LogMessage, SpawnedProcess } from "../shared/interfaces";
1313
import { PackageMap } from "../shared/pub/package_map";
14-
import { errorString, flatMap, notUndefined, throttle, uniq, uriToFilePath } from "../shared/utils";
14+
import { errorString, flatMap, notUndefined, PromiseCompleter, throttle, uniq, uriToFilePath } from "../shared/utils";
1515
import { sortBy } from "../shared/utils/array";
1616
import { applyColor, grey, grey2 } from "../shared/utils/colors";
1717
import { getRandomInt } from "../shared/utils/fs";
@@ -36,7 +36,7 @@ const messageWithUriPattern = new RegExp(`(.*?)((?:package|dart|file):.*\\.dart)
3636
// completionsRequest(response: DebugProtocol.CompletionsResponse, args: DebugProtocol.CompletionsArguments): void;
3737
export class DartDebugSession extends DebugSession {
3838
// TODO: Tidy all this up
39-
protected childProcess?: SpawnedProcess;
39+
protected childProcess?: SpawnedProcess | RemoteEditorTerminal;
4040
protected additionalPidsToTerminate: number[] = [];
4141
// We normally track the pid from Observatory to terminate the VM afterwards, but for Flutter Run it's
4242
// a remote PID and therefore doesn't make sense to try and terminate.
@@ -57,20 +57,15 @@ export class DartDebugSession extends DebugSession {
5757
private logStream?: fs.WriteStream;
5858
public debugSdkLibraries = false;
5959
public debugExternalLibraries = false;
60-
// Usually we buffer messages so we can scan whole lines for stack frames, but
61-
// for terminal apps that accept input this can result in the rendered text
62-
// being out of order if messages don't end with newlines. Since we don't support
63-
// linking up stack frames for terminal anyway, it's safe to just disable the
64-
// buffering there.
65-
public allowMessageBuffering = true;
66-
public sendOutputAsCustomEvent = false;
6760
public showDartDeveloperLogs = true;
6861
public useFlutterStructuredErrors = false;
6962
public evaluateGettersInDebugViews = false;
7063
protected previewToStringInDebugViews = false;
7164
protected useWriteServiceInfo = false;
7265
protected vmServiceInfoFile?: string;
7366
private serviceInfoPollTimer?: NodeJS.Timer;
67+
private remoteEditorTerminalLaunchCompleter?: PromiseCompleter<RemoteEditorTerminal>;
68+
private remoteEditorTerminalClosedCompleter?: PromiseCompleter<{ code: number | undefined }>;
7469
public debuggerHandlesPathsEverywhereForBreakpoints = false;
7570
protected threadManager: ThreadManager;
7671
public packageMap?: PackageMap;
@@ -79,7 +74,7 @@ export class DartDebugSession extends DebugSession {
7974
protected parseObservatoryUriFromStdOut: boolean = true;
8075
protected requiresProgram: boolean = true;
8176
protected pollforMemoryMs?: number; // If set, will poll for memory usage and send events back.
82-
protected processExit: Promise<void> = Promise.resolve();
77+
protected processExit: Promise<{ code: number | null, signal: string | null }> = Promise.resolve({ code: 0, signal: null });
8378
protected maxLogLineLength: number = 1000; // This should always be overriden in launch/attach requests but we have it here for narrower types.
8479
protected shouldKillProcessOnTerminate = true;
8580
protected logCategory = LogCategory.General; // This isn't used as General, since both debuggers override it.
@@ -117,7 +112,7 @@ export class DartDebugSession extends DebugSession {
117112
this.sendResponse(response);
118113
}
119114

120-
protected launchRequest(response: DebugProtocol.LaunchResponse, args: DartLaunchRequestArguments): void {
115+
protected async launchRequest(response: DebugProtocol.LaunchResponse, args: DartLaunchRequestArguments): Promise<void> {
121116
if (!args || !args.dartPath || (this.requiresProgram && !args.program)) {
122117
this.logToUser("Unable to restart debugging. Please try ending the debug session and starting again.\n");
123118
this.sendEvent(new TerminatedEvent());
@@ -149,43 +144,51 @@ export class DartDebugSession extends DebugSession {
149144
this.evaluateGettersInDebugViews = args.evaluateGettersInDebugViews;
150145
this.previewToStringInDebugViews = args.previewToStringInDebugViews;
151146
this.useWriteServiceInfo = args.useWriteServiceInfo;
152-
this.sendOutputAsCustomEvent = args.console === "terminal";
153-
this.allowMessageBuffering = args.console !== "terminal";
154147
this.debuggerHandlesPathsEverywhereForBreakpoints = args.debuggerHandlesPathsEverywhereForBreakpoints;
155148
this.logFile = args.observatoryLogFile;
156149
this.maxLogLineLength = args.maxLogLineLength;
157150

158151
this.sendResponse(response);
159152

160-
this.childProcess = this.spawnProcess(args);
161-
162-
// Start watching for the service info file.
163-
if (this.vmServiceInfoFile) {
153+
if (this.useWriteServiceInfo) {
154+
this.parseObservatoryUriFromStdOut = false;
155+
this.vmServiceInfoFile = path.join(os.tmpdir(), `dart-vm-service-${getRandomInt(0x1000, 0x10000).toString(16)}.json`);
164156
this.startServiceFilePolling();
165157
}
166158

167-
const process = this.childProcess;
168-
this.processExited = false;
169-
this.processExit = new Promise((resolve) => process.on("exit", resolve));
170-
process.stdout.setEncoding("utf8");
171-
process.stdout.on("data", (data) => {
172-
let match: RegExpExecArray | null = null;
173-
if (this.shouldConnectDebugger && this.parseObservatoryUriFromStdOut && !this.observatory) {
174-
match = observatoryListeningBannerPattern.exec(data.toString());
175-
}
176-
if (match) {
177-
this.initDebugger(this.websocketUriForObservatoryUri(match[1]));
178-
} else if (this.sendStdOutToConsole)
179-
this.logToUserBuffered(data.toString(), "stdout");
180-
});
181-
process.stderr.setEncoding("utf8");
182-
process.stderr.on("data", (data) => {
183-
this.logToUserBuffered(data.toString(), "stderr");
184-
});
185-
process.on("error", (error) => {
186-
this.logToUser(`${error}\n`, "stderr");
187-
});
188-
process.on("exit", async (code, signal) => {
159+
if (args.console === "terminal") {
160+
this.childProcess = await this.spawnRemoteEditorProcess(args);
161+
162+
this.remoteEditorTerminalClosedCompleter = new PromiseCompleter<{ code: number | undefined }>();
163+
this.processExit = this.remoteEditorTerminalClosedCompleter.promise.then((({ code }) => {
164+
return { code: code !== undefined ? code : null, signal: null };
165+
}));
166+
} else {
167+
const process = this.spawnProcess(args);
168+
169+
this.childProcess = process;
170+
this.processExited = false;
171+
this.processExit = new Promise((resolve) => process.on("exit", (code, signal) => resolve({ code, signal })));
172+
process.stdout.setEncoding("utf8");
173+
process.stdout.on("data", (data) => {
174+
let match: RegExpExecArray | null = null;
175+
if (this.shouldConnectDebugger && this.parseObservatoryUriFromStdOut && !this.observatory) {
176+
match = observatoryListeningBannerPattern.exec(data.toString());
177+
}
178+
if (match) {
179+
this.initDebugger(this.websocketUriForObservatoryUri(match[1]));
180+
} else if (this.sendStdOutToConsole)
181+
this.logToUserBuffered(data.toString(), "stdout");
182+
});
183+
process.stderr.setEncoding("utf8");
184+
process.stderr.on("data", (data) => {
185+
this.logToUserBuffered(data.toString(), "stderr");
186+
});
187+
process.on("error", (error) => {
188+
this.logToUser(`${error}\n`, "stderr");
189+
});
190+
}
191+
this.processExit.then(async ({ code, signal }) => {
189192
this.stopServiceFilePolling();
190193
this.processExited = true;
191194
this.log(`Process exited (${signal ? `${signal}`.toLowerCase() : code})`);
@@ -252,14 +255,45 @@ export class DartDebugSession extends DebugSession {
252255
}
253256

254257
protected spawnProcess(args: DartLaunchRequestArguments) {
258+
const appArgs = this.buildAppArgs(args);
259+
260+
this.log(`Spawning ${args.dartPath} with args ${JSON.stringify(appArgs)}`);
261+
if (args.cwd)
262+
this.log(`.. in ${args.cwd}`);
263+
const process = safeSpawn(args.cwd, args.dartPath, appArgs, args.env);
264+
265+
this.log(` PID: ${process.pid}`);
266+
267+
return process;
268+
}
269+
270+
protected async spawnRemoteEditorProcess(args: DartLaunchRequestArguments): Promise<RemoteEditorTerminal> {
271+
const appArgs = this.buildAppArgs(args);
272+
273+
this.log(`Spawning ${args.dartPath} remotely with args ${JSON.stringify(appArgs)}`);
274+
if (args.cwd)
275+
this.log(`.. in ${args.cwd}`);
276+
277+
this.remoteEditorTerminalLaunchCompleter = new PromiseCompleter<RemoteEditorTerminal>();
278+
this.sendEvent(new Event("dart.startTerminalProcess", {
279+
args: appArgs,
280+
binPath: args.dartPath,
281+
cwd: args.cwd,
282+
env: args.env,
283+
}));
284+
285+
this.log(` PID: ${process.pid}`);
286+
287+
return this.remoteEditorTerminalLaunchCompleter.promise;
288+
}
289+
290+
private buildAppArgs(args: DartLaunchRequestArguments) {
255291
let appArgs = [];
256292
if (this.shouldConnectDebugger) {
257293
appArgs.push(`--enable-vm-service=${args.vmServicePort}`);
258294
appArgs.push("--pause_isolates_on_start=true");
259295
}
260296
if (this.useWriteServiceInfo) {
261-
this.parseObservatoryUriFromStdOut = false;
262-
this.vmServiceInfoFile = path.join(os.tmpdir(), `dart-vm-service-${getRandomInt(0x1000, 0x10000).toString(16)}.json`);
263297
appArgs.push(`--write-service-info=${this.vmServiceInfoFile}`);
264298
appArgs.push("-DSILENT_OBSERVATORY=true");
265299
}
@@ -273,16 +307,7 @@ export class DartDebugSession extends DebugSession {
273307
if (args.args) {
274308
appArgs = appArgs.concat(args.args);
275309
}
276-
277-
this.log(`Spawning ${args.dartPath} with args ${JSON.stringify(appArgs)}`);
278-
if (args.cwd)
279-
this.log(`.. in ${args.cwd}`);
280-
281-
const process = safeSpawn(args.cwd, args.dartPath, appArgs, args.env);
282-
283-
this.log(` PID: ${process.pid}`);
284-
285-
return process;
310+
return appArgs;
286311
}
287312

288313
protected websocketUriForObservatoryUri(uri: string) {
@@ -334,7 +359,7 @@ export class DartDebugSession extends DebugSession {
334359
}
335360
}
336361

337-
private tryReadServiceFile() {
362+
private async tryReadServiceFile(): Promise<void> {
338363
if (!this.vmServiceInfoFile || !fs.existsSync(this.vmServiceInfoFile))
339364
return;
340365

@@ -358,6 +383,12 @@ export class DartDebugSession extends DebugSession {
358383
url.pathname = `${url.pathname}/ws`;
359384

360385
this.stopServiceFilePolling();
386+
// Ensure we don't try to start anything before we've finished
387+
// setting up the process when running remotely.
388+
if (this.remoteEditorTerminalLaunchCompleter) {
389+
await this.remoteEditorTerminalLaunchCompleter.promise;
390+
await new Promise((resolve) => setTimeout(resolve, 0));
391+
}
361392
this.initDebugger(url.toString());
362393
} catch (e) {
363394
this.logger.error(e, LogCategory.Observatory);
@@ -542,12 +573,16 @@ export class DartDebugSession extends DebugSession {
542573
}
543574
}
544575
if (!this.processExited) {
545-
try {
546-
this.log(`${request}: Terminating main process with ${signal}...`);
547-
this.childProcess.kill(signal);
548-
} catch (e) {
549-
// This tends to throw a lot because the shell process quit when we terminated the related
550-
// VM process above, so just swallow the error.
576+
if (this.childProcess.pid) {
577+
try {
578+
this.log(`${request}: Terminating main process with ${signal}...`);
579+
process.kill(this.childProcess.pid, signal);
580+
} catch (e) {
581+
// This tends to throw a lot because the shell process quit when we terminated the related
582+
// VM process above, so just swallow the error.
583+
}
584+
} else {
585+
this.log(`${request}: Process had no PID.`);
551586
}
552587
} else {
553588
this.log(`${request}: Main process had already quit.`);
@@ -1282,17 +1317,28 @@ export class DartDebugSession extends DebugSession {
12821317
await this.callService(args.type, args.params);
12831318
this.sendResponse(response);
12841319
break;
1285-
case "dart.userInput":
1286-
if (this.childProcess && !this.childProcess.killed && !this.processExited)
1287-
this.childProcess.stdin.write(args.input);
1288-
this.sendResponse(response);
1289-
break;
12901320
case "updateDebugOptions":
12911321
this.debugExternalLibraries = !!args.debugExternalLibraries;
12921322
this.debugSdkLibraries = !!args.debugSdkLibraries;
12931323
await this.threadManager.setLibrariesDuggableForAllIsolates();
12941324
this.sendResponse(response);
12951325
break;
1326+
case "remoteEditorTerminalLaunch":
1327+
if (!this.remoteEditorTerminalLaunchCompleter) {
1328+
this.log(`Invalid ${request} because no completer`, LogSeverity.Warn);
1329+
return;
1330+
}
1331+
this.remoteEditorTerminalLaunchCompleter.resolve(new RemoteEditorTerminal(args.pid));
1332+
this.sendResponse(response);
1333+
break;
1334+
case "remoteEditorTerminalClosed":
1335+
if (!this.remoteEditorTerminalClosedCompleter) {
1336+
this.log(`Invalid ${request} because no completer`, LogSeverity.Warn);
1337+
return;
1338+
}
1339+
this.remoteEditorTerminalClosedCompleter.resolve({ code: args?.code });
1340+
this.sendResponse(response);
1341+
break;
12961342
// Flutter requests that may be sent during test runs or other places
12971343
// that we don't currently support. TODO: Fix this by moving all the
12981344
// service extension stuff out of Flutter to here, and making it not
@@ -1893,11 +1939,6 @@ export class DartDebugSession extends DebugSession {
18931939
/// [5:01:50 PM] [General] [Info] [stderr] main (file:///D:/a/
18941940
/// [5:01:50 PM] [General] [Info] [stderr] Dart-Code/Dart-Code/src/test/test_projects/hello_world/bin/broken.dart:2:3)
18951941
protected logToUserBuffered(message: string, category: string) {
1896-
if (!this.allowMessageBuffering) {
1897-
this.logToUser(message, category);
1898-
return;
1899-
}
1900-
19011942
this.logBuffer[category] = this.logBuffer[category] || "";
19021943
this.logBuffer[category] += message;
19031944

@@ -1913,10 +1954,6 @@ export class DartDebugSession extends DebugSession {
19131954
// Logs a message back to the editor. Does not add its own newlines, you must
19141955
// provide them!
19151956
protected logToUser(message: string, category?: string, colorText = (s: string) => s) {
1916-
if (this.sendOutputAsCustomEvent) {
1917-
this.sendEvent(new Event("dart.output", { message, category }));
1918-
return;
1919-
}
19201957
// Extract stack frames from the message so we can do nicer formatting of them.
19211958
const frame = this.getStackFrameData(message) || this.getWebStackFrameData(message) || this.getMessageWithUriData(message);
19221959

@@ -1974,3 +2011,9 @@ interface MessageWithUriData {
19742011
prefix: string;
19752012
sourceUri: string;
19762013
}
2014+
2015+
class RemoteEditorTerminal {
2016+
public killed = false;
2017+
2018+
constructor(public readonly pid: number) { }
2019+
}

‎src/debug/flutter_debug_impl.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export class FlutterDebugSession extends DartDebugSession {
5454
super.initializeRequest(response, args);
5555
}
5656

57-
protected launchRequest(response: DebugProtocol.LaunchResponse, args: FlutterLaunchRequestArguments): void {
57+
protected async launchRequest(response: DebugProtocol.LaunchResponse, args: FlutterLaunchRequestArguments): Promise<void> {
5858
this.flutterTrackWidgetCreation = args && args.flutterTrackWidgetCreation;
5959
this.outputCategory = "stdout";
6060
return super.launchRequest(response, args);
@@ -64,7 +64,7 @@ export class FlutterDebugSession extends DartDebugSession {
6464
// For flutter attach, we actually do the same thing as launch - we run a flutter process
6565
// (flutter attach instead of flutter run).
6666
// this.observatoryUriIsProbablyReconnectable = true;
67-
this.launchRequest(response, args);
67+
return this.launchRequest(response, args);
6868
}
6969

7070
protected spawnProcess(args: FlutterLaunchRequestArguments): any {

0 commit comments

Comments
 (0)