Skip to content

Commit

Permalink
Require explicit spawn wrap mode
Browse files Browse the repository at this point in the history
  • Loading branch information
demurgos committed May 10, 2019
1 parent 48d97e6 commit a6a78ba
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 78 deletions.
53 changes: 37 additions & 16 deletions src/lib/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,34 @@ import { RootProcess, SwOptions } from "./types";
const DEFAULT_SHIM_ROOT_NAME = ".node-spawn-wrap";
const SHIM_ROOT_ENV_VAR = "SPAWN_WRAP_SHIM_ROOT";

export enum SwMode {
/**
* Run the wrapped module in the same process as the wrapper.
*
* A `runMain` function will be available on the wrapper API.
* Changing the exec arguments will have no effect.
* The patched spawn calls will attempt to reduce the number of intermediate
* processes. At most one intermediate process may be created to ensure that
* the user exec args are applied.
* Spawned processes without a main script will not be wrapped (e.g. `--eval`,
* `--interactive`, etc.)
*/
SameProcess = "same-process",

/**
* Run the wrapped module in a subprocess of the wrapper.
*
* The wrapper must take care to manually spawn the subprocess. It gets access
* to its arguments through the wrapper API. It must also register the
* internal patcher manually (through `--require ...`) to continue watching
* the subtree.
* All the node processes can be wrapped this way.
* This mode requires more work but grants more control other the
* subprocesses.
*/
SubProcess = "sub-process",
}

/**
* Spawn wrap context.
*/
Expand All @@ -27,7 +55,7 @@ export interface SwContext<D = any> {
/**
* Absolute system path for the corresponding dependencies.
*/
readonly deps: Readonly<Record<"debug" | "foregroundChild" | "isWindows" | "pathEnvVar" | "parseNodeOptions" | "signalExit", string>>;
readonly deps: Readonly<Record<"debug" | "foregroundChild" | "pathEnvVar" | "parseNodeOptions" | "signalExit", string>>;

/**
* Unique key identifying this context.
Expand Down Expand Up @@ -80,17 +108,11 @@ export interface SwContext<D = any> {
readonly data: D;

/**
* Run the wrapper and child process in the same process.
*
* Using the same process allows to reduce memory usage and improve speed
* but prevents changing the node engine flags (such as
* `--experimental-modules`) dynamically inside the wrapper.
* If the spawned node process uses node engine flags, multiple processes
* may be used.
* Controls if the wrapped and wrapper modules run in the same process or not.
*
* Default: `true`.
* See `SwMode` documentation for more information about each mode.
*/
readonly sameProcess: boolean;
readonly mode: SwMode;

/**
* Information about the root process.
Expand All @@ -108,7 +130,7 @@ interface ResolvedOptions {
data: any;
key: string;
shimDir: string;
sameProcess: boolean;
mode: SwMode;
}

export function withWrapContext<R = any>(options: SwOptions, handler: (ctx: SwContext) => Promise<R>): Promise<R> {
Expand Down Expand Up @@ -170,7 +192,7 @@ function realpathMkdirpSync(path: string): string {
}

/**
* Retuns the default shim root.
* Returns the default shim root.
*
* If the environment variable `SPAWN_WRAP_SHIM_ROOT` is defined, it returns
* its value. Otherwise, it returns the directory `.node-spawn-wrap` in the
Expand Down Expand Up @@ -232,9 +254,9 @@ function resolveOptions(options: SwOptions): ResolvedOptions {
);

const wrapper = path.resolve(options.wrapper);
const mode: SwMode = options.mode;
const data = options.data !== undefined ? JSON.parse(JSON.stringify(options.data)) : {};
const shimRoot = options.shimRoot !== undefined ? path.resolve(options.shimRoot) : getShimRoot();
const sameProcess: boolean = options.sameProcess !== undefined ? options.sameProcess : true;

debug("resolveOptions wrapper=%j data=%j shimRoot=%j", wrapper, data, shimRoot);

Expand All @@ -246,7 +268,7 @@ function resolveOptions(options: SwOptions): ResolvedOptions {
data,
key,
shimDir,
sameProcess,
mode,
};
}

Expand All @@ -256,7 +278,6 @@ function resolvedOptionsToContext(resolved: ResolvedOptions): SwContext {
deps: Object.freeze({
debug: require.resolve("./debug"),
foregroundChild: require.resolve("demurgos-foreground-child"),
isWindows: require.resolve("is-windows"),
pathEnvVar: require.resolve("./path-env-var"),
parseNodeOptions: require.resolve("./parse-node-options"),
signalExit: require.resolve("signal-exit"),
Expand All @@ -268,7 +289,7 @@ function resolvedOptionsToContext(resolved: ResolvedOptions): SwContext {
preloadScript: path.join(resolved.shimDir, "preload.js"),
wrapper: resolved.wrapper,
data: resolved.data,
sameProcess: resolved.sameProcess,
mode: resolved.mode,
root: Object.freeze({
execPath: process.execPath,
pid: process.pid,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { SwContext } from "./context";
export { SwContext, SwMode } from "./context";
export { withSpawnWrap, withSpawnWrapSync } from "./local";
export { ChildProcessProxy, ReadableStreamProxy, observeSpawn, ObserveSpawnOptions, SpawnEvent } from "./observable/index";
export { patchInternalsWithContext, patchInternals } from "./internals";
Expand Down
31 changes: 19 additions & 12 deletions src/lib/mungers/node.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SwContext } from "../context";
import { SwContext, SwMode } from "../context";
import { debug } from "../debug";
import { getExeBasename } from "../exe-type";
import { ParsedNodeOptions, parseNodeOptions } from "../parse-node-options";
Expand All @@ -9,18 +9,25 @@ export function mungeNode(ctx: SwContext, options: NormalizedOptions): Normalize
const parsed: ParsedNodeOptions = parseNodeOptions(options.args);

let newArgs: string[];
if (ctx.sameProcess) {
if (parsed.appArgs.length > 0) {
// Has a script
newArgs = [parsed.execPath, ...parsed.execArgs, "--", ctx.shimScript, ...parsed.appArgs];
} else {
// `--interactive`, `--eval`, `--version`, etc.
// Avoid wrapping these kind of invocations in same-process mode.
newArgs = [...options.args];
switch (ctx.mode) {
case SwMode.SameProcess: {
if (parsed.appArgs.length > 0) {
// Has a script
newArgs = [parsed.execPath, ...parsed.execArgs, "--", ctx.shimScript, ...parsed.appArgs];
} else {
// `--interactive`, `--eval`, `--version`, etc.
// Avoid wrapping these kind of invocations in same-process mode.
newArgs = [...options.args];
}
break;
}
} else {
// In subProcess mode, the exec args are not applied to the wrapper process.
newArgs = [parsed.execPath, "--", ctx.shimScript, ...parsed.execArgs, ...parsed.appArgs];
case SwMode.SubProcess: {
// In subProcess mode, the exec args are not applied to the wrapper process.
newArgs = [parsed.execPath, "--", ctx.shimScript, ...parsed.execArgs, ...parsed.appArgs];
break;
}
default:
throw new Error(`Unknown mode: ${ctx.mode}`);
}

let newFile: string = options.file;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/observable/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import cp, { ChildProcess } from "child_process";
import events from "events";
import { Observable, Observer, Subscribable, Unsubscribable } from "rxjs";
import { filter } from "rxjs/operators";
import { SwMode } from "../context";
import { Api, withSpawnWrap, WithSwOptions } from "../local";
import { SwOptions } from "../types";
import { ClientMessage, InfoMessage, StreamEvent } from "./protocol";
import { RemoteSpawnClient, SpawnServer } from "./server";

Expand Down Expand Up @@ -267,7 +267,7 @@ export function observeSpawn(
host: server.host,
port: server.port,
},
sameProcess: false,
mode: SwMode.SubProcess,
api,
};

Expand Down
74 changes: 40 additions & 34 deletions src/lib/templates/shim-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const {debug} = require(context.deps.debug);
const {removeFromPathEnv, isPathEnvName} = require(context.deps.pathEnvVar);
const {parseNodeOptions} = require(context.deps.parseNodeOptions);
const foregroundChild = require(context.deps.foregroundChild);
const isWindows = require(context.deps.isWindows);
const spawnWrap = require(context.module);
const Module = require("module");
const path = require("path");
Expand Down Expand Up @@ -51,40 +50,47 @@ function shimMain() {

let args: ReadonlyArray<string>;

if (context.sameProcess) {
// user args: no exec args and no shim path
const userArgs: ReadonlyArray<string> = [process.argv[0]].concat(originalArgs);
const parsed: any = parseNodeOptions(userArgs);
if (parsed.appArgs.length === 0 || parsed.hasEval || parsed.hasInteractive) {
// Avoid running the wrapper in same process mode if node is used without a script
// Can happen for example if we intercept `node -e <...>`
debug("no main file!");
foregroundChild(process.execPath, originalArgs);
return;
switch (context.mode) {
case "same-process": {
// user args: no exec args and no shim path
const userArgs: ReadonlyArray<string> = [process.argv[0]].concat(originalArgs);
const parsed: any = parseNodeOptions(userArgs);
if (parsed.appArgs.length === 0 || parsed.hasEval || parsed.hasInteractive) {
// Avoid running the wrapper in same process mode if node is used without a script
// Can happen for example if we intercept `node -e <...>`
debug("no main file!");
foregroundChild(process.execPath, originalArgs);
return;
}
if (parsed.execArgs.length > 0) {
// `process.argv` starts with some non-applied exec args: we need to spawn
// a subprocess to apply them.
const fixedArgs: ReadonlyArray<string> = [
...withoutTrailingDoubleDash(process.execArgv),
parsed.execArgs,
"--",
__filename,
...parsed.appArgs,
];
foregroundChild(process.execPath, fixedArgs);
return;
}
// If we reached this point, it means that we were called as:
// `/path/to/node ...execArgs /path/to/shim ...userAppArgs`
args = [...originalArgs];
break;
}
if (parsed.execArgs.length > 0) {
// `process.argv` starts with some non-applied exec args: we need to spawn
// a subprocess to apply them.
const fixedArgs: ReadonlyArray<string> = [
...withoutTrailingDoubleDash(process.execArgv),
parsed.execArgs,
"--",
__filename,
...parsed.appArgs,
];
foregroundChild(process.execPath, fixedArgs);
return;
case "sub-process": {
// `process.execArgv` should be empty or `--` so we only pass the user args.
// Which will contain the user exec args and app args.
// If we wanted to add it, it should be passed to withoutTrailingDoubleDash
// first to ensure that the user exec args are still applied.
args = [...originalArgs];
break;
}
default: {
throw new Error(`Unknown mode: ${context.mode}`);
}
// If we reached this point, it means that we were called as:
// `/path/to/node ...execArgs /path/to/shim ...userAppArgs`
args = [...originalArgs];
} else {
// Subprocess mode
// `process.execArgv` should be empty or `--` so we only pass the user args.
// Which will contain the user exec args and app args.
// If we wanted to add it, it should be passed to withoutTrailingDoubleDash
// first to ensure that the user exec args are still applied.
args = [...originalArgs];
}

// This will be insert again when a process is spawned through a patched spawn.
Expand All @@ -98,7 +104,7 @@ function shimMain() {
// Replace the shim script with the wrapper so it looks like the main
process.argv.splice(1, 1, context.wrapper);

if (context.sameProcess) {
if (context.mode === "same-process") {
spawnWrap.patchInternalsWithContext(context);

function runMain(): void {
Expand Down
9 changes: 5 additions & 4 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import cp from "child_process";
import { SwContext } from "./context";
import { SwContext, SwMode } from "./context";

export interface RootProcess {
pid: number;
Expand Down Expand Up @@ -45,10 +45,11 @@ export interface SwOptions {
shimRoot?: string;

/**
* Try to run the wrapper and original main in the same process.
* If `true`, then `WrapperApi` will have a `runMain`, otherwise not.
* Run the wrapped and wrapper modules in the same process or not.
*
* See `SwMode` documentation for more details.
*/
sameProcess?: boolean;
mode: SwMode;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test/basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const npmFixture = require.resolve('./fixtures/npm')

const WRAPPER = require.resolve('./fixtures/basic.wrapper.js')

var unwrap = sw.patchInternals({ wrapper: WRAPPER })
var unwrap = sw.patchInternals({ wrapper: WRAPPER, mode: 'same-process' })

var expect = 'WRAP ["--","{{FIXTURE}}","xyz"]\n' +
'["--"]\n' +
Expand Down
4 changes: 2 additions & 2 deletions test/double-wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ switch (process.argv[2]) {
case 'main':
console.error('main', process.pid, process.execArgv.concat(argv))
console.log('main')
sw.patchInternals({ wrapper: WRAPPER, data: 'first' })
sw.patchInternals({ wrapper: WRAPPER, data: 'first', mode: 'same-process' })
fg(node, [__filename, 'parent'])
break
case 'parent':
console.error('parent', process.pid, process.execArgv.concat(argv))
console.log('parent')
sw.patchInternals({ wrapper: WRAPPER, data: 'second' })
sw.patchInternals({ wrapper: WRAPPER, data: 'second', mode: 'same-process' })
fg(node, [__filename, 'child'])
break
case 'child':
Expand Down
2 changes: 1 addition & 1 deletion test/exec-flag.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const spawn = cp.spawn
const nodes = ['node', process.execPath]
const WRAPPER = require.resolve('./fixtures/exec-flag.wrapper.js')

sw.patchInternals({ wrapper: WRAPPER })
sw.patchInternals({ wrapper: WRAPPER, mode: 'same-process' })

t.test('try to wrap a -e invocation but it isnt wrapped', function (t) {
nodes.forEach(function (node) {
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const { spawn } = require('child_process')
const path = require('path')
const sw = require('../../')

sw.patchInternals({ wrapper: require.resolve('./test-shim.js') })
sw.patchInternals({ wrapper: require.resolve('./test-shim.js'), mode: 'same-process' })

spawn(path.resolve(process.argv[2]), process.argv.slice(3), {
stdio: 'inherit'
Expand Down
4 changes: 2 additions & 2 deletions test/local.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ function test () {
t.test('withSpawnWrapSync', function (t) {
t.plan(4)

const result = sw.withSpawnWrapSync({ wrapper: WRAPPER, data: { name: 'foo' } }, (fooApi) => {
return sw.withSpawnWrapSync({ wrapper: WRAPPER, data: { name: 'bar' } }, (barApi) => {
const result = sw.withSpawnWrapSync({ wrapper: WRAPPER, data: { name: 'foo' }, mode: 'same-process' }, (fooApi) => {
return sw.withSpawnWrapSync({ wrapper: WRAPPER, data: { name: 'bar' }, mode: 'same-process' }, (barApi) => {
{
const { stdout, stderr } = fooApi.spawnSync(process.execPath, [echoArgs, '1'])
const out = stdout.toString('UTF-8')
Expand Down
4 changes: 2 additions & 2 deletions test/wrap-twice.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ const WRAPPER = require.resolve('./fixtures/wrap-twice.wrapper.js')
switch (process.argv[2]) {
case 'main':
console.log('main')
sw.patchInternals({ wrapper: WRAPPER, data: 'outer' })
sw.patchInternals({ wrapper: WRAPPER, data: 'inner' })
sw.patchInternals({ wrapper: WRAPPER, data: 'outer', mode: 'same-process' })
sw.patchInternals({ wrapper: WRAPPER, data: 'inner', mode: 'same-process' })
fg(node, [__filename, 'parent'])
break

Expand Down

0 comments on commit a6a78ba

Please sign in to comment.