Skip to content

Commit 4a80bcb

Browse files
code-ashercode-asher
authored
Make everything use active evals (coder#30)
* Add trace log level * Use active eval to implement spdlog * Split server/client active eval interfaces Since all properties are *not* valid on both sides * +200% fire resistance * Implement exec using active evaluations * Fully implement child process streams * Watch impl, move child_process back to explicitly adding events Automatically forwarding all events might be the right move, but wanna think/discuss it a bit more because it didn't come out very cleanly. * Would you like some args with that callback? * Implement the rest of child_process using active evals * Rampant memory leaks Emit "kill" to active evaluations when client disconnects in order to kill processes. Most likely won't be the final solution. * Resolve some minor issues with output panel * Implement node-pty with active evals * Provide clearTimeout to vm sandbox * Implement socket with active evals * Extract some callback logic Also remove some eval interfaces, need to re-think those. * Implement net.Server and remainder of net.Socket using active evals * Implement dispose for active evaluations * Use trace for express requests * Handle sending buffers through evaluation events * Make event logging a bit more clear * Fix some errors due to us not actually instantiating until connect/listen * is this a commit message? * We can just create the evaluator in the ctor Not sure what I was thinking. * memory leak for you, memory leak for everyone * it's a ternary now * Don't dispose automatically on close or error The code may or may not be disposable at that point. * Handle parsing buffers on the client side as well * Remove unused protobuf * Remove TypedValue * Remove unused forkProvider and test * Improve dispose pattern for active evals * Socket calls close after error; no need to bind both * Improve comment * Comment is no longer wishy washy due to explicit boolean * Simplify check for sendHandle and options * Replace _require with __non_webpack_require__ Webpack will then replace this with `require` which we then provide to the vm sandbox. * Provide path.parse * Prevent original-fs from loading * Start with a pid of -1 vscode immediately checks the PID to see if the debug process launch correctly, but of course we don't get the pid synchronously. * Pass arguments to bootstrap-fork * Fully implement streams Was causing errors because internally the stream would set this.writing to true and it would never become false, so subsequent messages would never send. * Fix serializing errors and streams emitting errors multiple times * Was emitting close to data * Fix missing path for spawned processes * Move evaluation onDispose call Now it's accurate and runs when the active evaluation has actually disposed. * Fix promisifying fs.exists * Fix some active eval callback issues * Patch existsSync in debug adapter
1 parent 7376201 commit 4a80bcb

39 files changed

Lines changed: 1770 additions & 8807 deletions

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
"mini-css-extract-plugin": "^0.5.0",
2929
"node-sass": "^4.11.0",
3030
"npm-run-all": "^4.1.5",
31+
"path-browserify": "^1.0.0",
3132
"preload-webpack-plugin": "^3.0.0-beta.2",
3233
"sass-loader": "^7.1.0",
3334
"string-replace-loader": "^2.1.1",
@@ -48,6 +49,7 @@
4849
},
4950
"dependencies": {
5051
"node-loader": "^0.6.0",
52+
"spdlog": "^0.7.2",
5153
"trash": "^4.3.0",
5254
"webpack-merge": "^4.2.1"
5355
}

‎packages/ide/src/fill/child_process.ts‎

Lines changed: 172 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,85 +1,194 @@
11
import * as cp from "child_process";
2-
import { Client, useBuffer } from "@coder/protocol";
2+
import * as net from "net";
3+
import * as stream from "stream";
4+
import { CallbackEmitter, ActiveEvalReadable, ActiveEvalWritable, createUniqueEval } from "./evaluation";
35
import { client } from "./client";
4-
import { promisify } from "./util";
6+
import { promisify } from "util";
57

6-
class CP {
7-
public constructor(
8-
private readonly client: Client,
9-
) { }
8+
declare var __non_webpack_require__: typeof require;
109

11-
public exec = (
12-
command: string,
13-
options?: { encoding?: BufferEncoding | string | "buffer" | null } & cp.ExecOptions | null | ((error: Error | null, stdout: string, stderr: string) => void) | ((error: Error | null, stdout: Buffer, stderr: Buffer) => void),
14-
callback?: ((error: Error | null, stdout: string, stderr: string) => void) | ((error: Error | null, stdout: Buffer, stderr: Buffer) => void),
15-
): cp.ChildProcess => {
16-
// TODO: Probably should add an `exec` instead of using `spawn`, especially
17-
// since bash might not be available.
18-
const childProcess = this.client.spawn("bash", ["-c", command.replace(/"/g, "\\\"")]);
10+
class ChildProcess extends CallbackEmitter implements cp.ChildProcess {
11+
private _connected: boolean = false;
12+
private _killed: boolean = false;
13+
private _pid = -1;
14+
public readonly stdin: stream.Writable;
15+
public readonly stdout: stream.Readable;
16+
public readonly stderr: stream.Readable;
17+
// We need the explicit type otherwise TypeScript thinks it is (Writable | Readable)[].
18+
public readonly stdio: [stream.Writable, stream.Readable, stream.Readable] = [this.stdin, this.stdout, this.stderr];
1919

20-
let stdout = "";
21-
childProcess.stdout.on("data", (data) => {
22-
stdout += data.toString();
23-
});
20+
// tslint:disable no-any
21+
public constructor(method: "exec", command: string, options?: { encoding?: string | null } & cp.ExecOptions | null, callback?: (...args: any[]) => void);
22+
public constructor(method: "fork", modulePath: string, options?: cp.ForkOptions, args?: string[]);
23+
public constructor(method: "spawn", command: string, options?: cp.SpawnOptions, args?: string[]);
24+
public constructor(method: "exec" | "spawn" | "fork", command: string, options: object = {}, callback?: string[] | ((...args: any[]) => void)) {
25+
// tslint:enable no-any
26+
super();
2427

25-
let stderr = "";
26-
childProcess.stderr.on("data", (data) => {
27-
stderr += data.toString();
28-
});
28+
let args: string[] = [];
29+
if (Array.isArray(callback)) {
30+
args = callback;
31+
callback = undefined;
32+
}
2933

30-
childProcess.on("exit", (exitCode) => {
31-
const error = exitCode !== 0 ? new Error(stderr) : null;
32-
if (typeof options === "function") {
33-
callback = options;
34-
}
35-
if (callback) {
36-
// @ts-ignore not sure how to make this work.
37-
callback(
38-
error,
39-
useBuffer(options) ? Buffer.from(stdout) : stdout,
40-
useBuffer(options) ? Buffer.from(stderr) : stderr,
41-
);
34+
this.ae = client.run((ae, command, method, args, options, callbackId) => {
35+
const cp = __non_webpack_require__("child_process") as typeof import("child_process");
36+
const { maybeCallback, createUniqueEval, bindWritable, bindReadable, preserveEnv } = __non_webpack_require__("@coder/ide/src/fill/evaluation") as typeof import("@coder/ide/src/fill/evaluation");
37+
38+
preserveEnv(options);
39+
40+
let childProcess: cp.ChildProcess;
41+
switch (method) {
42+
case "exec":
43+
childProcess = cp.exec(command, options, maybeCallback(ae, callbackId));
44+
break;
45+
case "spawn":
46+
childProcess = cp.spawn(command, args, options);
47+
break;
48+
case "fork":
49+
const forkOptions = options as cp.ForkOptions;
50+
if (forkOptions && forkOptions.env && forkOptions.env.AMD_ENTRYPOINT) {
51+
// TODO: This is vscode-specific and should be abstracted.
52+
const { forkModule } = __non_webpack_require__("@coder/server/src/vscode/bootstrapFork") as typeof import ("@coder/server/src/vscode/bootstrapFork");
53+
childProcess = forkModule(forkOptions.env.AMD_ENTRYPOINT, args, forkOptions);
54+
} else {
55+
childProcess = cp.fork(command, args, options);
56+
}
57+
break;
58+
default:
59+
throw new Error(`invalid method ${method}`);
4260
}
61+
62+
ae.on("disconnect", () => childProcess.disconnect());
63+
ae.on("kill", (signal) => childProcess.kill(signal));
64+
ae.on("ref", () => childProcess.ref());
65+
ae.on("send", (message, callbackId) => childProcess.send(message, maybeCallback(ae, callbackId)));
66+
ae.on("unref", () => childProcess.unref());
67+
68+
ae.emit("pid", childProcess.pid);
69+
childProcess.on("close", (code, signal) => ae.emit("close", code, signal));
70+
childProcess.on("disconnect", () => ae.emit("disconnect"));
71+
childProcess.on("error", (error) => ae.emit("error", error));
72+
childProcess.on("exit", (code, signal) => ae.emit("exit", code, signal));
73+
childProcess.on("message", (message) => ae.emit("message", message));
74+
75+
bindWritable(createUniqueEval(ae, "stdin"), childProcess.stdin);
76+
bindReadable(createUniqueEval(ae, "stdout"), childProcess.stdout);
77+
bindReadable(createUniqueEval(ae, "stderr"), childProcess.stderr);
78+
79+
return {
80+
onDidDispose: (cb): cp.ChildProcess => childProcess.on("close", cb),
81+
dispose: (): void => {
82+
childProcess.kill();
83+
setTimeout(() => childProcess.kill("SIGKILL"), 5000); // Double tap.
84+
},
85+
};
86+
}, command, method, args, options, this.storeCallback(callback));
87+
88+
this.ae.on("pid", (pid) => {
89+
this._pid = pid;
90+
this._connected = true;
4391
});
4492

45-
// @ts-ignore TODO: not fully implemented
46-
return childProcess;
93+
this.stdin = new ActiveEvalWritable(createUniqueEval(this.ae, "stdin"));
94+
this.stdout = new ActiveEvalReadable(createUniqueEval(this.ae, "stdout"));
95+
this.stderr = new ActiveEvalReadable(createUniqueEval(this.ae, "stderr"));
96+
97+
this.ae.on("close", (code, signal) => this.emit("close", code, signal));
98+
this.ae.on("disconnect", () => this.emit("disconnect"));
99+
this.ae.on("error", (error) => this.emit("error", error));
100+
this.ae.on("exit", (code, signal) => {
101+
this._connected = false;
102+
this._killed = true;
103+
this.emit("exit", code, signal);
104+
});
105+
this.ae.on("message", (message) => this.emit("message", message));
106+
}
107+
108+
public get pid(): number { return this._pid; }
109+
public get connected(): boolean { return this._connected; }
110+
public get killed(): boolean { return this._killed; }
111+
112+
public kill(): void { this.ae.emit("kill"); }
113+
public disconnect(): void { this.ae.emit("disconnect"); }
114+
public ref(): void { this.ae.emit("ref"); }
115+
public unref(): void { this.ae.emit("unref"); }
116+
117+
public send(
118+
message: any, // tslint:disable-line no-any to match spec
119+
sendHandle?: net.Socket | net.Server | ((error: Error) => void),
120+
options?: cp.MessageOptions | ((error: Error) => void),
121+
callback?: (error: Error) => void): boolean {
122+
if (typeof sendHandle === "function") {
123+
callback = sendHandle;
124+
sendHandle = undefined;
125+
} else if (typeof options === "function") {
126+
callback = options;
127+
options = undefined;
128+
}
129+
if (sendHandle || options) {
130+
throw new Error("sendHandle and options are not supported");
131+
}
132+
this.ae.emit("send", message, this.storeCallback(callback));
133+
134+
// Unfortunately this will always have to be true since we can't retrieve
135+
// the actual response synchronously.
136+
return true;
137+
}
138+
}
139+
140+
class CP {
141+
public readonly ChildProcess = ChildProcess;
142+
143+
public exec = (
144+
command: string,
145+
options?: { encoding?: string | null } & cp.ExecOptions | null | ((error: cp.ExecException | null, stdout: string, stderr: string) => void) | ((error: cp.ExecException | null, stdout: Buffer, stderr: Buffer) => void),
146+
callback?: ((error: cp.ExecException | null, stdout: string, stderr: string) => void) | ((error: cp.ExecException | null, stdout: Buffer, stderr: Buffer) => void),
147+
): cp.ChildProcess => {
148+
if (typeof options === "function") {
149+
callback = options;
150+
options = undefined;
151+
}
152+
153+
return new ChildProcess("exec", command, options, callback);
47154
}
48155

49156
public fork = (modulePath: string, args?: string[] | cp.ForkOptions, options?: cp.ForkOptions): cp.ChildProcess => {
50-
if (options && options.env && options.env.AMD_ENTRYPOINT) {
51-
// @ts-ignore TODO: not fully implemented
52-
return this.client.bootstrapFork(
53-
options.env.AMD_ENTRYPOINT,
54-
Array.isArray(args) ? args : [],
55-
// @ts-ignore TODO: env is a different type
56-
Array.isArray(args) || !args ? options : args,
57-
);
157+
if (args && !Array.isArray(args)) {
158+
options = args;
159+
args = undefined;
58160
}
59161

60-
// @ts-ignore TODO: not fully implemented
61-
return this.client.fork(
62-
modulePath,
63-
Array.isArray(args) ? args : [],
64-
// @ts-ignore TODO: env is a different type
65-
Array.isArray(args) || !args ? options : args,
66-
);
162+
return new ChildProcess("fork", modulePath, options, args);
67163
}
68164

69165
public spawn = (command: string, args?: string[] | cp.SpawnOptions, options?: cp.SpawnOptions): cp.ChildProcess => {
70-
// @ts-ignore TODO: not fully implemented
71-
return this.client.spawn(
72-
command,
73-
Array.isArray(args) ? args : [],
74-
// @ts-ignore TODO: env is a different type
75-
Array.isArray(args) || !args ? options : args,
76-
);
166+
if (args && !Array.isArray(args)) {
167+
options = args;
168+
args = undefined;
169+
}
170+
171+
return new ChildProcess("spawn", command, options, args);
77172
}
78173
}
79174

80-
const fillCp = new CP(client);
81-
82-
// tslint:disable-next-line no-any makes util.promisify return an object
83-
(fillCp as any).exec[promisify.customPromisifyArgs] = ["stdout", "stderr"];
84-
175+
const fillCp = new CP();
176+
// Methods that don't follow the standard callback pattern (an error followed
177+
// by a single result) need to provide a custom promisify function.
178+
Object.defineProperty(fillCp.exec, promisify.custom, {
179+
value: (
180+
command: string,
181+
options?: { encoding?: string | null } & cp.ExecOptions | null,
182+
): Promise<{ stdout: string | Buffer, stderr: string | Buffer }> => {
183+
return new Promise((resolve, reject): void => {
184+
fillCp.exec(command, options, (error: cp.ExecException | null, stdout: string | Buffer, stderr: string | Buffer) => {
185+
if (error) {
186+
reject(error);
187+
} else {
188+
resolve({ stdout, stderr });
189+
}
190+
});
191+
});
192+
},
193+
});
85194
export = fillCp;

‎packages/ide/src/fill/electron.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ import { IKey, Dialog as DialogBox } from "./dialog";
66
import { clipboard } from "./clipboard";
77
import { client } from "./client";
88

9-
// Use this to get around Webpack inserting our fills.
10-
declare var _require: typeof require;
9+
declare var __non_webpack_require__: typeof require;
1110

1211
// tslint:disable-next-line no-any
1312
(global as any).getOpenUrls = (): string[] => {
@@ -99,7 +98,7 @@ class Clipboard {
9998
class Shell {
10099
public async moveItemToTrash(path: string): Promise<void> {
101100
await client.evaluate((path) => {
102-
const trash = _require("trash") as typeof import("trash");
101+
const trash = __non_webpack_require__("trash") as typeof import("trash");
103102

104103
return trash(path);
105104
}, path);

0 commit comments

Comments
 (0)