Skip to content

Commit 37538dc

Browse files
committed
refactor(compass-shell): Refactor child process creation to avoid writing to fs; Use worker in child process to work around vm SIGINT issue
1 parent e4b7e5f commit 37538dc

File tree

4 files changed

+62
-26
lines changed

4 files changed

+62
-26
lines changed

packages/compass-shell/src/modules/runtime.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function reduceSetupRuntime(state, action) {
6969
const newRuntime = new WorkerRuntime(url, options);
7070

7171
// TODO: For testing purposes
72-
window.restart = newRuntime.restart.bind(newRuntime);
72+
window.shellRuntime = newRuntime;
7373

7474
return {
7575
error: action.error,
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { SHARE_ENV, Worker } from 'worker_threads';
2+
import { exposeAll, createCaller } from './rpc';
3+
4+
import workerSrc from 'inline-entry-loader!./worker';
5+
6+
const workerProcess = new Worker(workerSrc, { eval: true, env: SHARE_ENV });
7+
8+
const worker = createCaller(
9+
['init', 'evaluate', 'getCompletions', 'ping'],
10+
workerProcess
11+
);
12+
13+
const evaluationListener = createCaller(
14+
['onPrint', 'onPrompt', 'toggleTelemetry'],
15+
process
16+
);
17+
18+
exposeAll(worker, process);
19+
exposeAll(evaluationListener, workerProcess);
20+
21+
workerProcess.once('message', () => {
22+
process.nextTick(() => {
23+
process.send('ready');
24+
});
25+
});

packages/compass-shell/src/modules/worker-runtime/runtime.js

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,12 @@
1-
import path from 'path';
2-
import fs from 'fs';
3-
// eslint-disable-next-line camelcase
4-
import child_process from 'child_process';
5-
import crypto from 'crypto';
6-
import electron from 'electron';
7-
import worker from 'inline-entry-loader!./worker';
1+
import { once } from 'events';
2+
import { spawn } from 'child_process';
83
import { createCaller, exposeAll } from './rpc';
94

10-
function createChildFromSource(src) {
11-
const tmpFile = path.resolve(
12-
(electron.app || electron.remote.app).getPath('temp'),
13-
`worker-runtime-${crypto.randomBytes(32).toString('hex')}`
14-
);
5+
import childSrc from 'inline-entry-loader!./child';
156

16-
// eslint-disable-next-line no-sync
17-
fs.writeFileSync(tmpFile, src, 'utf8');
18-
19-
const childProcess = child_process.spawn(process.execPath, [tmpFile], {
20-
stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
7+
async function createChildFromSource(src) {
8+
const childProcess = spawn(process.execPath, [], {
9+
stdio: ['pipe', 'inherit', 'inherit', 'ipc'],
2110
env: { ...process.env, ELECTRON_RUN_AS_NODE: 1 },
2211
/**
2312
* This would be awesome, but only supported since node 12.16.0, compass
@@ -27,22 +16,34 @@ function createChildFromSource(src) {
2716
serialization: 'advanced',
2817
});
2918

19+
childProcess.stdin.write(src);
20+
childProcess.stdin.end();
21+
22+
// First message is a "ready" message
23+
await once(childProcess, 'message');
24+
3025
return childProcess;
3126
}
3227

3328
class WorkerRuntime {
3429
constructor(uri, options, cliOptions) {
3530
this.initOptions = { uri, options, cliOptions };
31+
// Creating worker is an async process, we want to "lock"
32+
// evaluate/getCompletions methods until worker is initiated from the get-go
33+
this.initPromiseResolve = null;
34+
this.initPromise = new Promise((resolve) => {
35+
this.initPromiseResolve = resolve;
36+
});
3637
this.evaluationListener = null;
3738
this.cancelEvaluate = () => {};
3839
this.initWorker();
3940
}
4041

41-
initWorker() {
42+
async initWorker() {
4243
const { uri, options, cliOptions } = this.initOptions;
43-
this.childProcess = createChildFromSource(worker);
44+
this.childProcess = await createChildFromSource(childSrc);
4445
this.worker = createCaller(
45-
['init', 'evaluate', 'getCompletions'],
46+
['init', 'evaluate', 'getCompletions', 'ping'],
4647
this.childProcess
4748
);
4849
this.workerEvaluationListener = exposeAll(
@@ -68,7 +69,7 @@ class WorkerRuntime {
6869
},
6970
this.childProcess
7071
);
71-
this.initPromise = this.worker.init(uri, options, cliOptions);
72+
this.initPromiseResolve(await this.worker.init(uri, options, cliOptions));
7273
}
7374

7475
async evaluate(code) {
@@ -108,18 +109,23 @@ class WorkerRuntime {
108109
return prev;
109110
}
110111

112+
interruptWorker() {
113+
this.childProcess.kill('SIGINT');
114+
}
115+
111116
terminateWorker() {
112117
this.cancelEvaluate();
113118
this.childProcess.kill();
119+
this.childProcess = null;
114120
}
115121

116-
restart() {
122+
restartWorker() {
117123
this.terminateWorker();
118124
this.initWorker();
119125
}
120126

121127
waitForCancelEvaluate() {
122-
return new Promise((resolve, reject) => {
128+
return new Promise((_resolve, reject) => {
123129
this.cancelEvaluate = () => {
124130
reject(new Error('Script execution was interrupted'));
125131
};

packages/compass-shell/src/modules/worker-runtime/worker.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { parentPort } from 'worker_threads';
12
import { ElectronRuntime } from '@mongosh/browser-runtime-electron';
23
import { CliServiceProvider } from '@mongosh/service-provider-server';
34
import { exposeAll, createCaller } from './rpc';
@@ -8,7 +9,7 @@ let runtime = null;
89

910
const evaluationListener = createCaller(
1011
['onPrint', 'onPrompt', 'toggleTelemetry'],
11-
process
12+
parentPort
1213
);
1314

1415
async function init(uri, options, cliOptions) {
@@ -56,4 +57,8 @@ async function getCompletions(code) {
5657

5758
const worker = { init, evaluate, getCompletions };
5859

59-
exposeAll(worker, process);
60+
exposeAll(worker, parentPort);
61+
62+
process.nextTick(() => {
63+
parentPort.postMessage('ready');
64+
});

0 commit comments

Comments
 (0)