Skip to content

Commit caf86fb

Browse files
code-asherkylecarbs
authored andcommitted
Get search working and clean up disconnected client (#23)
* Use ipc instead of pipe * Run callback passed to child process's send method * It also returns true * Correct send signature * Kill processes when client disconnects
1 parent f7e8fad commit caf86fb

File tree

6 files changed

+49
-31
lines changed

6 files changed

+49
-31
lines changed

packages/protocol/src/browser/command.ts

+9-3
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ export interface ChildProcess {
3030

3131
kill(signal?: string): void;
3232

33-
send(message: string | Uint8Array, ipc?: false): void;
34-
send(message: any, ipc: true): void;
33+
send(message: string | Uint8Array, callback?: () => void, ipc?: false): void;
34+
send(message: any, callback: undefined | (() => void), ipc: true): void;
3535

3636
on(event: "message", listener: (data: any) => void): void;
3737
on(event: "error", listener: (err: Error) => void): void;
@@ -101,7 +101,7 @@ export class ServerProcess extends events.EventEmitter implements ChildProcess {
101101
this._connected = false;
102102
}
103103

104-
public send(message: string | Uint8Array | any, ipc: boolean = this.ipc): void {
104+
public send(message: string | Uint8Array | any, callback?: (error: Error | null) => void, ipc: boolean = this.ipc): boolean {
105105
const send = new WriteToSessionMessage();
106106
send.setId(this.id);
107107
send.setSource(ipc ? WriteToSessionMessage.Source.IPC : WriteToSessionMessage.Source.STDIN);
@@ -113,6 +113,12 @@ export class ServerProcess extends events.EventEmitter implements ChildProcess {
113113
const client = new ClientMessage();
114114
client.setWriteToSession(send);
115115
this.connection.send(client.serializeBinary());
116+
// TODO: properly implement?
117+
if (callback) {
118+
callback(null);
119+
}
120+
121+
return true;
116122
}
117123

118124
public resize(dimensions: TTYDimensions): void {

packages/protocol/src/node/command.ts

+11-7
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@ export interface Process {
1313
stdin?: stream.Writable;
1414
stdout?: stream.Readable;
1515
stderr?: stream.Readable;
16+
send?: (message: string) => void;
1617

1718
pid: number;
1819
killed?: boolean;
1920

20-
on(event: "data", cb: (data: string) => void): void;
21+
on(event: "data" | "message", cb: (data: string) => void): void;
2122
on(event: "exit", listener: (exitCode: number, signal?: number) => void): void;
2223
write(data: string | Uint8Array): void;
2324
resize?(cols: number, rows: number): void;
@@ -61,7 +62,7 @@ export const handleNewSession = (connection: SendableConnection, newSession: New
6162
connection.send(sm.serializeBinary());
6263
}
6364
}, 200);
64-
65+
6566
ptyProc.on("exit", () => {
6667
clearTimeout(timer);
6768
});
@@ -93,6 +94,9 @@ export const handleNewSession = (connection: SendableConnection, newSession: New
9394
stderr: proc.stderr,
9495
stdout: proc.stdout,
9596
stdio: proc.stdio,
97+
send: (message): void => {
98+
proc.send(message);
99+
},
96100
on: (...args: any[]): void => ((proc as any).on)(...args), // tslint:disable-line no-any
97101
write: (d): boolean => proc.stdin.write(d),
98102
kill: (s): void => proc.kill(s || "SIGTERM"),
@@ -105,7 +109,7 @@ export const handleNewSession = (connection: SendableConnection, newSession: New
105109

106110
let data = msg.toString();
107111
if (_source === SessionOutputMessage.Source.IPC) {
108-
data = Buffer.from(msg.toString(), "base64").toString();
112+
// data = Buffer.from(msg.toString(), "base64").toString();
109113
}
110114

111115
return [
@@ -139,10 +143,10 @@ export const handleNewSession = (connection: SendableConnection, newSession: New
139143
});
140144
}
141145

142-
if (process.stdio && process.stdio[3]) {
143-
// We have ipc fd
144-
process.stdio[3].on("data", (data) => {
145-
sendOutput(SessionOutputMessage.Source.IPC, data);
146+
// IPC.
147+
if (process.send) {
148+
process.on("message", (data) => {
149+
sendOutput(SessionOutputMessage.Source.IPC, JSON.stringify(data));
146150
});
147151
}
148152

packages/protocol/src/node/server.ts

+20-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as os from "os";
22
import * as cp from "child_process";
33
import * as path from "path";
4-
import { mkdir, WriteStream } from "fs";
4+
import { mkdir } from "fs";
55
import { promisify } from "util";
66
import { TextDecoder } from "text-encoding";
77
import { logger, field } from "@coder/logger";
@@ -37,6 +37,17 @@ export class Server {
3737
logger.error("Failed to handle client message", field("length", data.byteLength), field("exception", ex));
3838
}
3939
});
40+
connection.onClose(() => {
41+
this.sessions.forEach((s) => {
42+
s.kill();
43+
});
44+
this.connections.forEach((c) => {
45+
c.destroy();
46+
});
47+
this.servers.forEach((s) => {
48+
s.close();
49+
});
50+
});
4051

4152
if (!options) {
4253
logger.warn("No server options provided. InitMessage will not be sent.");
@@ -97,7 +108,12 @@ export class Server {
97108
private handleMessage(message: ClientMessage): void {
98109
if (message.hasNewEval()) {
99110
const evalMessage = message.getNewEval()!;
100-
logger.debug("EvalMessage", field("id", evalMessage.getId()));
111+
logger.debug(() => [
112+
"EvalMessage",
113+
field("id", evalMessage.getId()),
114+
field("args", evalMessage.getArgsList()),
115+
field("function", evalMessage.getFunction()),
116+
]);
101117
evaluate(this.connection, evalMessage);
102118
} else if (message.hasNewSession()) {
103119
const sessionMessage = message.getNewSession()!;
@@ -141,10 +157,10 @@ export class Server {
141157
const data = new TextDecoder().decode(writeToSessionMessage.getData_asU8());
142158
const source = writeToSessionMessage.getSource();
143159
if (source === WriteToSessionMessage.Source.IPC) {
144-
if (!s.stdio || !s.stdio[3]) {
160+
if (!s.send) {
145161
throw new Error("Cannot send message via IPC to process without IPC");
146162
}
147-
(s.stdio[3] as WriteStream).write(data);
163+
s.send(JSON.parse(data));
148164
} else {
149165
s.write(data);
150166
}

packages/protocol/test/command.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ describe("spawn", () => {
1515
workingDirectory: "",
1616
forkProvider: (msg): cp.ChildProcess => {
1717
return cp.spawn(msg.getCommand(), msg.getArgsList(), {
18-
stdio: [null, null, null, "pipe"],
18+
stdio: [null, null, null, "ipc"],
1919
});
2020
},
2121
});
@@ -144,7 +144,7 @@ describe("spawn", () => {
144144
expect(msg.bananas).toBeTruthy();
145145
proc.kill();
146146
});
147-
proc.send({ bananas: true }, true);
147+
proc.send({ bananas: true }, undefined, true);
148148
proc.on("exit", () => done());
149149
});
150150
});

packages/server/src/vscode/bootstrapFork.ts

+3-15
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,18 @@
11
import * as cp from "child_process";
22
import * as fs from "fs";
3-
import * as net from "net";
43
import * as path from "path";
54

65
export const requireModule = (modulePath: string): void => {
76
process.env.AMD_ENTRYPOINT = modulePath;
87

98
const xml = require("xhr2");
109

11-
(<any>global).XMLHttpRequest = xml.XMLHttpRequest;
10+
// tslint:disable-next-line no-any this makes installing extensions work.
11+
(global as any).XMLHttpRequest = xml.XMLHttpRequest;
1212

1313
// Always do this so we can see console.logs.
1414
// process.env.VSCODE_ALLOW_IO = "true";
1515

16-
if (!process.send) {
17-
const socket = new net.Socket({ fd: 3 });
18-
socket.on("data", (data) => {
19-
process.emit("message", JSON.parse(data.toString()), undefined);
20-
});
21-
22-
// tslint:disable-next-line no-any
23-
process.send = (message: any): void => {
24-
socket.write(JSON.stringify(message));
25-
};
26-
}
27-
2816
const content = fs.readFileSync(path.join(process.env.BUILD_DIR as string || path.join(__dirname, "../.."), "./build/bootstrap-fork.js"));
2917
eval(content.toString());
3018
};
@@ -45,7 +33,7 @@ export const forkModule = (modulePath: string, env?: NodeJS.ProcessEnv): cp.Chil
4533
args.push("--env", JSON.stringify(env));
4634
}
4735
const options: cp.SpawnOptions = {
48-
stdio: [null, null, null, "pipe"],
36+
stdio: [null, null, null, "ipc"],
4937
};
5038
if (process.env.CLI === "true") {
5139
proc = cp.spawn(process.execPath, args, options);

packages/web/yarn.lock

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
2+
# yarn lockfile v1
3+
4+

0 commit comments

Comments
 (0)