From 3cc59ecac91a3433f47339de6eb936339a9c57b2 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 24 Jun 2022 13:51:25 +0000 Subject: [PATCH] Fix ESM node processes being unable to fork into other scripts Currently, Node processes instantiated through the `--esm` flag result in a child process being created so that the ESM loader can be registered. This works fine and is reasonable. The child process approach to register ESM hooks currently prevents the NodeJS `fork` method from being used because the `execArgv` propagated into forked processes causes `ts-node` (which is also propagated as child exec script -- this is good because it allows nested type resolution to work) to always execute the original entry-point, causing potential infinite loops because the designated fork module script is not executed as expected. This commit fixes this by not encoding the entry-point information into the state that is captured as part of the `execArgv`. Instead the entry-point information is always retrieved from the parsed rest command line arguments in the final stage (`phase4`). Fixes #1812. --- src/bin.ts | 122 +++++++++++++----- src/child/child-entrypoint.ts | 3 +- src/test/esm-loader.spec.ts | 40 ++++++ .../index.mts | 23 ++++ .../tsconfig.json | 5 + .../worker.mts | 3 + .../process-forking-nested-esm/index.ts | 23 ++++ .../process-forking-nested-esm/package.json | 3 + .../process-forking-nested-esm/tsconfig.json | 5 + .../process-forking-nested-esm/worker.ts | 3 + .../process-forking/index.ts | 23 ++++ .../process-forking/package.json | 3 + .../process-forking/tsconfig.json | 5 + .../process-forking/worker.js | 1 + 14 files changed, 229 insertions(+), 33 deletions(-) create mode 100644 tests/esm-child-process/process-forking-nested-esm-node-next/index.mts create mode 100644 tests/esm-child-process/process-forking-nested-esm-node-next/tsconfig.json create mode 100644 tests/esm-child-process/process-forking-nested-esm-node-next/worker.mts create mode 100644 tests/esm-child-process/process-forking-nested-esm/index.ts create mode 100644 tests/esm-child-process/process-forking-nested-esm/package.json create mode 100644 tests/esm-child-process/process-forking-nested-esm/tsconfig.json create mode 100644 tests/esm-child-process/process-forking-nested-esm/worker.ts create mode 100644 tests/esm-child-process/process-forking/index.ts create mode 100644 tests/esm-child-process/process-forking/package.json create mode 100644 tests/esm-child-process/process-forking/tsconfig.json create mode 100644 tests/esm-child-process/process-forking/worker.js diff --git a/src/bin.ts b/src/bin.ts index 8b5f91767..af784670a 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -48,7 +48,7 @@ export function main( const state: BootstrapState = { shouldUseChildProcess: false, isInChildProcess: false, - entrypoint: __filename, + tsNodeScript: __filename, parseArgvResult: args, }; return bootstrap(state); @@ -62,7 +62,7 @@ export function main( export interface BootstrapState { isInChildProcess: boolean; shouldUseChildProcess: boolean; - entrypoint: string; + tsNodeScript: string; parseArgvResult: ReturnType; phase2Result?: ReturnType; phase3Result?: ReturnType; @@ -319,28 +319,16 @@ Options: process.exit(0); } - // Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint - // This is complicated because node's behavior is complicated - // `node -e code -i ./script.js` ignores -e - const executeEval = code != null && !(interactive && restArgs.length); - const executeEntrypoint = !executeEval && restArgs.length > 0; - const executeRepl = - !executeEntrypoint && - (interactive || (process.stdin.isTTY && !executeEval)); - const executeStdin = !executeEval && !executeRepl && !executeEntrypoint; - const cwd = cwdArg || process.cwd(); - /** Unresolved. May point to a symlink, not realpath. May be missing file extension */ - const scriptPath = executeEntrypoint ? resolve(cwd, restArgs[0]) : undefined; - if (esm) payload.shouldUseChildProcess = true; + // If ESM is explicitly enabled through the flag, stage3 should be run in a child process + // with the ESM loaders configured. + if (esm) { + payload.shouldUseChildProcess = true; + } + return { - executeEval, - executeEntrypoint, - executeRepl, - executeStdin, cwd, - scriptPath, }; } @@ -372,7 +360,18 @@ function phase3(payload: BootstrapState) { esm, experimentalSpecifierResolution, } = payload.parseArgvResult; - const { cwd, scriptPath } = payload.phase2Result!; + const { cwd } = payload.phase2Result!; + + // NOTE: When we transition to a child process for ESM, the entry-point script determined + // here might not be the one used later in `phase4`. This can happen when we execute the + // original entry-point but then the process forks itself using e.g. `child_process.fork`. + // We will always use the original TS project in forked processes anyway, so it is + // expected and acceptable to retrieve the entry-point information here in `phase2`. + // See: https://github.com/TypeStrong/ts-node/issues/1812. + const { entryPointPath } = getEntryPointInfo( + payload.parseArgvResult!, + payload.phase2Result! + ); const preloadedConfig = findAndReadConfig({ cwd, @@ -387,7 +386,12 @@ function phase3(payload: BootstrapState) { compilerHost, ignore, logError, - projectSearchDir: getProjectSearchDir(cwd, scriptMode, cwdMode, scriptPath), + projectSearchDir: getProjectSearchDir( + cwd, + scriptMode, + cwdMode, + entryPointPath + ), project, skipProject, skipIgnore, @@ -403,23 +407,76 @@ function phase3(payload: BootstrapState) { experimentalSpecifierResolution as ExperimentalSpecifierResolution, }); - if (preloadedConfig.options.esm) payload.shouldUseChildProcess = true; + // If ESM is enabled through the parsed tsconfig, stage4 should be run in a child + // process with the ESM loaders configured. + if (preloadedConfig.options.esm) { + payload.shouldUseChildProcess = true; + } + return { preloadedConfig }; } +/** + * Determines the entry-point information from the argv and phase2 result. This + * method will be invoked in two places: + * + * 1. In phase 3 to be able to find a project from the potential entry-point script. + * 2. In phase 4 to determine the actual entry-point script. + * + * Note that we need to explicitly re-resolve the entry-point information in the final + * stage because the previous stage information could be modified when the bootstrap + * invocation transitioned into a child process for ESM. + * + * Stages before (phase 4) can and will be cached by the child process through the Brotli + * configuration and entry-point information is only reliable in the final phase. More + * details can be found in here: https://github.com/TypeStrong/ts-node/issues/1812. + */ +function getEntryPointInfo( + argvResult: NonNullable, + phase2Result: NonNullable +) { + const { code, interactive, restArgs } = argvResult; + const { cwd } = phase2Result; + + // Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint + // This is complicated because node's behavior is complicated + // `node -e code -i ./script.js` ignores -e + const executeEval = code != null && !(interactive && restArgs.length); + const executeEntrypoint = !executeEval && restArgs.length > 0; + const executeRepl = + !executeEntrypoint && + (interactive || (process.stdin.isTTY && !executeEval)); + const executeStdin = !executeEval && !executeRepl && !executeEntrypoint; + + /** Unresolved. May point to a symlink, not realpath. May be missing file extension */ + const entryPointPath = executeEntrypoint + ? resolve(cwd, restArgs[0]) + : undefined; + + return { + executeEval, + executeEntrypoint, + executeRepl, + executeStdin, + entryPointPath, + }; +} + function phase4(payload: BootstrapState) { - const { isInChildProcess, entrypoint } = payload; + const { isInChildProcess, tsNodeScript } = payload; const { version, showConfig, restArgs, code, print, argv } = payload.parseArgvResult; + const { cwd } = payload.phase2Result!; + const { preloadedConfig } = payload.phase3Result!; + const { + entryPointPath, + executeEntrypoint, executeEval, - cwd, - executeStdin, executeRepl, - executeEntrypoint, - scriptPath, - } = payload.phase2Result!; - const { preloadedConfig } = payload.phase3Result!; + executeStdin, + } = getEntryPointInfo(payload.parseArgvResult!, payload.phase2Result!); + /** * , [stdin], and [eval] are all essentially virtual files that do not exist on disc and are backed by a REPL * service to handle eval-ing of code. @@ -566,12 +623,13 @@ function phase4(payload: BootstrapState) { // Prepend `ts-node` arguments to CLI for child processes. process.execArgv.push( - entrypoint, + tsNodeScript, ...argv.slice(2, argv.length - restArgs.length) ); + // TODO this comes from BoostrapState process.argv = [process.argv[1]] - .concat(executeEntrypoint ? ([scriptPath] as string[]) : []) + .concat(executeEntrypoint ? ([entryPointPath] as string[]) : []) .concat(restArgs.slice(executeEntrypoint ? 1 : 0)); // Execute the main contents (either eval, script or piped). diff --git a/src/child/child-entrypoint.ts b/src/child/child-entrypoint.ts index 03a02d2e9..55d77ad9d 100644 --- a/src/child/child-entrypoint.ts +++ b/src/child/child-entrypoint.ts @@ -8,8 +8,9 @@ const base64Payload = base64ConfigArg.slice(argPrefix.length); const payload = JSON.parse( brotliDecompressSync(Buffer.from(base64Payload, 'base64')).toString() ) as BootstrapState; + payload.isInChildProcess = true; -payload.entrypoint = __filename; +payload.tsNodeScript = __filename; payload.parseArgvResult.argv = process.argv; payload.parseArgvResult.restArgs = process.argv.slice(3); diff --git a/src/test/esm-loader.spec.ts b/src/test/esm-loader.spec.ts index 41c421fd6..2becddb72 100644 --- a/src/test/esm-loader.spec.ts +++ b/src/test/esm-loader.spec.ts @@ -22,6 +22,7 @@ import { TEST_DIR, tsSupportsImportAssertions, tsSupportsResolveJsonModule, + tsSupportsStableNodeNextNode16, } from './helpers'; import { createExec, createSpawn, ExecReturn } from './exec-helpers'; import { join, resolve } from 'path'; @@ -358,6 +359,45 @@ test.suite('esm', (test) => { }); } + test.suite('esm child process and forking', (test) => { + test('should be able to fork vanilla NodeJS script', async () => { + const { err, stdout, stderr } = await exec( + `${BIN_PATH} --esm ./esm-child-process/process-forking/index.ts` + ); + + expect(err).toBe(null); + expect(stdout.trim()).toBe('Passing: from main'); + expect(stderr).toBe(''); + }); + + test('should be able to fork into a nested TypeScript ESM script', async () => { + const { err, stdout, stderr } = await exec( + `${BIN_PATH} --esm ./esm-child-process/process-forking-nested-esm/index.ts` + ); + + expect(err).toBe(null); + expect(stdout.trim()).toBe('Passing: from main'); + expect(stderr).toBe(''); + }); + + test.suite( + 'with NodeNext TypeScript resolution and `.mts` extension', + (test) => { + test.runIf(tsSupportsStableNodeNextNode16); + + test('should be able to fork into a nested TypeScript ESM script', async () => { + const { err, stdout, stderr } = await exec( + `${BIN_PATH} --esm ./esm-child-process/process-forking-nested-esm-node-next/index.mts` + ); + + expect(err).toBe(null); + expect(stdout.trim()).toBe('Passing: from main'); + expect(stderr).toBe(''); + }); + } + ); + }); + test.suite('parent passes signals to child', (test) => { test.runSerially(); diff --git a/tests/esm-child-process/process-forking-nested-esm-node-next/index.mts b/tests/esm-child-process/process-forking-nested-esm-node-next/index.mts new file mode 100644 index 000000000..e76286d8e --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-esm-node-next/index.mts @@ -0,0 +1,23 @@ +import { fork } from 'child_process'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; + +// Initially set the exit code to non-zero. We only set it to `0` when the +// worker process finishes properly with the expected stdout message. +process.exitCode = 1; + +const projectDir = dirname(fileURLToPath(import.meta.url)); +const workerProcess = fork(join(projectDir, 'worker.mts'), [], { + stdio: 'pipe', +}); + +let stdout = ''; + +workerProcess.stdout.on('data', (chunk) => (stdout += chunk.toString('utf8'))); +workerProcess.on('error', () => (process.exitCode = 1)); +workerProcess.on('close', (status, signal) => { + if (status === 0 && signal === null && stdout.trim() === 'Works') { + console.log('Passing: from main'); + process.exitCode = 0; + } +}); diff --git a/tests/esm-child-process/process-forking-nested-esm-node-next/tsconfig.json b/tests/esm-child-process/process-forking-nested-esm-node-next/tsconfig.json new file mode 100644 index 000000000..3998b5074 --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-esm-node-next/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "NodeNext" + } +} diff --git a/tests/esm-child-process/process-forking-nested-esm-node-next/worker.mts b/tests/esm-child-process/process-forking-nested-esm-node-next/worker.mts new file mode 100644 index 000000000..4114d5ab0 --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-esm-node-next/worker.mts @@ -0,0 +1,3 @@ +const message: string = 'Works'; + +console.log(message); diff --git a/tests/esm-child-process/process-forking-nested-esm/index.ts b/tests/esm-child-process/process-forking-nested-esm/index.ts new file mode 100644 index 000000000..d2ebe098a --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-esm/index.ts @@ -0,0 +1,23 @@ +import { fork } from 'child_process'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; + +// Initially set the exit code to non-zero. We only set it to `0` when the +// worker process finishes properly with the expected stdout message. +process.exitCode = 1; + +const projectDir = dirname(fileURLToPath(import.meta.url)); +const workerProcess = fork(join(projectDir, 'worker.ts'), [], { + stdio: 'pipe', +}); + +let stdout = ''; + +workerProcess.stdout.on('data', (chunk) => (stdout += chunk.toString('utf8'))); +workerProcess.on('error', () => (process.exitCode = 1)); +workerProcess.on('close', (status, signal) => { + if (status === 0 && signal === null && stdout.trim() === 'Works') { + console.log('Passing: from main'); + process.exitCode = 0; + } +}); diff --git a/tests/esm-child-process/process-forking-nested-esm/package.json b/tests/esm-child-process/process-forking-nested-esm/package.json new file mode 100644 index 000000000..3dbc1ca59 --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-esm/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/tests/esm-child-process/process-forking-nested-esm/tsconfig.json b/tests/esm-child-process/process-forking-nested-esm/tsconfig.json new file mode 100644 index 000000000..1ac61592b --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-esm/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "ESNext" + } +} diff --git a/tests/esm-child-process/process-forking-nested-esm/worker.ts b/tests/esm-child-process/process-forking-nested-esm/worker.ts new file mode 100644 index 000000000..4114d5ab0 --- /dev/null +++ b/tests/esm-child-process/process-forking-nested-esm/worker.ts @@ -0,0 +1,3 @@ +const message: string = 'Works'; + +console.log(message); diff --git a/tests/esm-child-process/process-forking/index.ts b/tests/esm-child-process/process-forking/index.ts new file mode 100644 index 000000000..52052eb72 --- /dev/null +++ b/tests/esm-child-process/process-forking/index.ts @@ -0,0 +1,23 @@ +import { fork } from 'child_process'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; + +// Initially set the exit code to non-zero. We only set it to `0` when the +// worker process finishes properly with the expected stdout message. +process.exitCode = 1; + +const projectDir = dirname(fileURLToPath(import.meta.url)); +const workerProcess = fork(join(projectDir, 'worker.js'), [], { + stdio: 'pipe', +}); + +let stdout = ''; + +workerProcess.stdout.on('data', (chunk) => (stdout += chunk.toString('utf8'))); +workerProcess.on('error', () => (process.exitCode = 1)); +workerProcess.on('close', (status, signal) => { + if (status === 0 && signal === null && stdout.trim() === 'Works') { + console.log('Passing: from main'); + process.exitCode = 0; + } +}); diff --git a/tests/esm-child-process/process-forking/package.json b/tests/esm-child-process/process-forking/package.json new file mode 100644 index 000000000..3dbc1ca59 --- /dev/null +++ b/tests/esm-child-process/process-forking/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/tests/esm-child-process/process-forking/tsconfig.json b/tests/esm-child-process/process-forking/tsconfig.json new file mode 100644 index 000000000..1ac61592b --- /dev/null +++ b/tests/esm-child-process/process-forking/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "ESNext" + } +} diff --git a/tests/esm-child-process/process-forking/worker.js b/tests/esm-child-process/process-forking/worker.js new file mode 100644 index 000000000..820d10b2e --- /dev/null +++ b/tests/esm-child-process/process-forking/worker.js @@ -0,0 +1 @@ +console.log('Works');