diff --git a/doc/api/process.md b/doc/api/process.md index 36e087b2cd0..0437a5744a7 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -1727,6 +1727,10 @@ This is achieved by using the `execve` POSIX function and therefore no memory or resources from the current process are preserved, except for the standard input, standard output and standard error file descriptor. +For standard input, standard output and standard error descriptors that still +refer to their original files or streams, the blocking mode returns to its state +at Node.js startup. Other file status flags and terminal settings remain unchanged. + On success, all other resources are discarded by the system when the processes are swapped, without triggering any exit or close events, without running any JavaScript cleanup handler (for example `process.on('exit')`), diff --git a/src/node.cc b/src/node.cc index e2efaad8a20..6a8a3ef54d7 100644 --- a/src/node.cc +++ b/src/node.cc @@ -667,6 +667,29 @@ static void PlatformInit(ProcessInitializationFlags::Flags flags) { #endif // _WIN32 } +#ifdef __POSIX__ +int GetStdioFlagsForExec(int fd, int flags) { + CHECK_GE(fd, 0); + CHECK_LE(fd, STDERR_FILENO); + if (init_process_flags.load() & + ProcessInitializationFlags::kNoStdioInitialization) { + return flags; + } + + const auto& original = stdio[fd]; + struct stat current; + if (fstat(fd, ¤t) == -1) return -1; + + // Only apply the startup mode while the descriptor refers to the same file. + if (original.stat.st_dev != current.st_dev || + original.stat.st_ino != current.st_ino) { + return flags; + } + + return (flags & ~O_NONBLOCK) | (original.flags & O_NONBLOCK); +} +#endif // __POSIX__ + // Safe to call more than once and from signal handlers. void ResetStdio() { if (init_process_flags.load() & diff --git a/src/node_internals.h b/src/node_internals.h index bb45d73e909..5b751a6fdb5 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -110,6 +110,7 @@ std::string FormatErrorMessage(v8::Isolate* isolate, bool add_source_line = true); void ResetStdio(); // Safe to call more than once and from signal handlers. #ifdef __POSIX__ +int GetStdioFlagsForExec(int fd, int flags); void SignalExit(int signal, siginfo_t* info, void* ucontext); #endif diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 6a35e5c5e4c..bc7d33ef7cf 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -511,22 +511,12 @@ static void ReallyExit(const FunctionCallbackInfo& args) { } #if defined __POSIX__ && !defined(__PASE__) -// Clears FD_CLOEXEC on `fd` so the descriptor is inherited across execve(2). -// On success returns the previous F_GETFD flags (>= 0) so callers can -// restore them if execve(2) subsequently fails. On failure returns -1 with -// errno set. -inline int persist_standard_stream(int fd) { - int flags = fcntl(fd, F_GETFD, 0); - - if (flags < 0) { - return flags; - } - - if (fcntl(fd, F_SETFD, flags & ~FD_CLOEXEC) < 0) { - return -1; - } - - return flags; +static int FcntlNoIntr(int fd, int command, int flags = 0) { + int result; + do { + result = fcntl(fd, command, flags); + } while (result == -1 && errno == EINTR); + return result; } static void Execve(const FunctionCallbackInfo& args) { @@ -579,41 +569,61 @@ static void Execve(const FunctionCallbackInfo& args) { envp[envp_array->Length()] = nullptr; - // Set stdin, stdout and stderr to be non-close-on-exec so that the new - // process will inherit them. Save the previous flags on each fd so we can - // restore them if execve(2) fails and we throw back to JS. - int saved_stdio_flags[3] = {-1, -1, -1}; + // Snapshot all descriptors before changing any flags: descriptors such as + // stdout and stderr may share an open file description (e.g. with 2>&1). + struct { + int descriptor_flags; + int status_flags; + int exec_flags; + } stdio_flags[3]; for (int fd = 0; fd < 3; fd++) { - int prev = persist_standard_stream(fd); - if (prev < 0) { - int fcntl_errno = errno; - // Undo changes already applied to earlier fds before throwing. - for (int j = 0; j < fd; j++) { - fcntl(j, F_SETFD, saved_stdio_flags[j]); - } - env->ThrowErrnoException(fcntl_errno, "fcntl"); + auto& flags = stdio_flags[fd]; + flags.descriptor_flags = FcntlNoIntr(fd, F_GETFD); + if (flags.descriptor_flags == -1) { + env->ThrowErrnoException(errno, "fcntl"); + return; + } + flags.status_flags = FcntlNoIntr(fd, F_GETFL); + if (flags.status_flags == -1) { + env->ThrowErrnoException(errno, "fcntl"); + return; + } + flags.exec_flags = GetStdioFlagsForExec(fd, flags.status_flags); + if (flags.exec_flags == -1) { + env->ThrowErrnoException(errno, "fstat"); return; } - saved_stdio_flags[fd] = prev; } - // Perform the execve operation. - // - // Note: we intentionally do not invoke RunAtExit(env) here. On success the - // kernel discards the current address space when loading the new image, so - // any in-memory side effects of AtExit callbacks are lost anyway. On - // failure we want to leave the environment intact so the thrown exception - // can be observed and handled by JS code. - execve(*executable, argv.data(), envp.data()); + auto restore_stdio = [&]() { + for (int fd = 0; fd < 3; fd++) { + const auto& flags = stdio_flags[fd]; + if (flags.exec_flags != flags.status_flags) { + FcntlNoIntr(fd, F_SETFL, flags.status_flags); + } + FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags); + } + }; - // If execve returned, it failed. Restore the FD_CLOEXEC flags we cleared - // above so that a failed call leaves no observable side effects, then - // throw an ErrnoException so JS can catch it. - int execve_errno = errno; for (int fd = 0; fd < 3; fd++) { - fcntl(fd, F_SETFD, saved_stdio_flags[fd]); + const auto& flags = stdio_flags[fd]; + if (FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags & ~FD_CLOEXEC) == -1 || + (flags.exec_flags != flags.status_flags && + FcntlNoIntr(fd, F_SETFL, flags.exec_flags) == -1)) { + const int err = errno; + restore_stdio(); + env->ThrowErrnoException(err, "fcntl"); + return; + } } - env->ThrowErrnoException(execve_errno, "execve", nullptr, *executable); + + // Do not run AtExit callbacks: a failed execve() must leave Node.js usable. + execve(*executable, argv.data(), envp.data()); + + // Error construction can invoke JavaScript setters. Restore stdio first. + const int err = errno; + restore_stdio(); + env->ThrowErrnoException(err, "execve", nullptr, *executable); } #endif diff --git a/test/fixtures/process-execve-stdio.py b/test/fixtures/process-execve-stdio.py new file mode 100644 index 00000000000..dd2d684ec36 --- /dev/null +++ b/test/fixtures/process-execve-stdio.py @@ -0,0 +1,100 @@ +import contextlib +import fcntl +import json +import os +import select +import socket +import subprocess +import sys + +# Node.js has no fcntl() API, and its startup can change inherited stdio flags. +# Use Python to prepare and inspect descriptors without those changes. The +# JavaScript test owns the test cases and checks the observations returned here. + + +def status_flags(fd): + return fcntl.fcntl(fd, fcntl.F_GETFL) + + +def nonblocking_mask(flags): + return sum(1 << fd for fd, flag in enumerate(flags) if flag & os.O_NONBLOCK) + + +def read_line(fd, timeout): + response = b'' + while not response.endswith(b'\n'): + ready, _, _ = select.select([fd], [], [], timeout) + assert ready, 'Timed out waiting for child response' + chunk = os.read(fd, 4096) + assert chunk, response + response += chunk + return response + + +def run_case(node, script, kind, mode, mask, alias, timeout): + with contextlib.ExitStack() as resources: + pairs = [] + for _ in range(3): + if kind == 'pipe': + pair = os.pipe() + else: + pair = tuple(endpoint.detach() for endpoint in socket.socketpair()) + for fd in pair: + resources.callback(os.close, fd) + pairs.append(pair) + + stdio = [pairs[0][0], pairs[1][1], pairs[2][1]] + if alias: + stdio[2] = stdio[1] + + for fd, stream in enumerate(stdio): + flags = status_flags(stream) + if mask & (1 << fd): + flags |= os.O_NONBLOCK + fcntl.fcntl(stream, fcntl.F_SETFL, flags) + + control_parent, control_child = socket.socketpair() + resources.enter_context(control_parent) + resources.enter_context(control_child) + args = [node, '--no-warnings', script, mode] + args += [sys.executable] if mode == 'exec' else [str(control_child.fileno())] + child = subprocess.Popen(args, stdin=stdio[0], stdout=stdio[1], + stderr=stdio[2], pass_fds=(control_child.fileno(),)) + control_child.close() + try: + if mode == 'exec': + result = {'flags': json.loads(read_line(pairs[1][0], timeout))} + else: + snapshots = [] + while True: + phase = read_line(control_parent.fileno(), timeout).decode().strip() + flags = [status_flags(fd) for fd in stdio] + snapshots.append({'phase': phase, 'flags': flags, + 'nonblocking': nonblocking_mask(flags)}) + control_parent.sendall(b'x') + if phase == 'after': + break + result = {'snapshots': snapshots} + result['status'] = child.wait(timeout=timeout) + return result + finally: + if child.poll() is None: + child.kill() + child.wait() + + +if sys.argv[1] == 'check': + flags = [status_flags(fd) for fd in range(3)] + close_on_exec = 0 + append = 0 + for fd in range(3): + if fcntl.fcntl(fd, fcntl.F_GETFD) & fcntl.FD_CLOEXEC: + close_on_exec |= 1 << fd + if flags[fd] & os.O_APPEND: + append |= 1 << fd + result = {'nonblocking': nonblocking_mask(flags), + 'closeOnExec': close_on_exec, 'append': append} + os.write(1, (json.dumps(result) + '\n').encode()) +else: + node, script, options = sys.argv[2:] + print(json.dumps(run_case(node, script, **json.loads(options)))) diff --git a/test/parallel/test-process-execve-stdio.js b/test/parallel/test-process-execve-stdio.js new file mode 100644 index 00000000000..85442e7a8f6 --- /dev/null +++ b/test/parallel/test-process-execve-stdio.js @@ -0,0 +1,124 @@ +'use strict'; + +const common = require('../common'); +const assert = require('node:assert'); +const { closeSync, constants, openSync, readFileSync, readSync, writeFileSync, writeSync } = require('node:fs'); +const { isMainThread } = require('node:worker_threads'); +const { spawnSyncAndAssert } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); + +if (!isMainThread) { + common.skip('process.execve is not available in Workers'); +} else if (common.isWindows || common.isIBMi) { + common.skip('process.execve is not available in Windows or IBM i'); +} + +const fixture = fixtures.path('process-execve-stdio.py'); +const mode = process.argv[2]; + +if (mode) { + // Opening the libuv handles makes the inherited pipes nonblocking. + assert.ok(process.stdin); + assert.ok(process.stdout); + assert.ok(process.stderr); + + if (mode === 'reopened') { + closeSync(1); + const flags = constants.O_WRONLY | constants.O_NONBLOCK | constants.O_APPEND; + assert.strictEqual(openSync(process.argv[4], flags), 1); + } + + if (mode === 'exec' || mode === 'reopened') { + const python = process.argv[3]; + process.execve(python, [python, fixture, 'check']); + assert.fail('process.execve unexpectedly returned'); + } + + // Keep the process alive while the parent checks the shared status flags. + // Normal process exit would restore them and conceal a failed rollback. + const control = Number(process.argv[3]); + function synchronize(phase) { + writeSync(control, `${phase}\n`); + assert.strictEqual(readSync(control, Buffer.alloc(1), 0, 1, null), 1); + } + + synchronize('before'); + if (mode === 'closed') closeSync(2); + Object.defineProperty(Error.prototype, 'code', { + configurable: true, + get: common.mustNotCall(), + set: common.mustCall(function(code) { + // Error construction can execute JavaScript before execve() throws. + // The descriptor flags must already be restored at this point. + synchronize('setter'); + Object.defineProperty(this, 'code', { + value: code, + writable: true, + enumerable: true, + configurable: true, + }); + }), + }); + try { + assert.throws(() => { + process.execve(`${process.execPath}.nonexistent`, []); + }, { + code: mode === 'closed' ? 'EBADF' : 'ENOENT', + syscall: mode === 'closed' ? 'fcntl' : 'execve', + }); + } finally { + delete Error.prototype.code; + } + + synchronize('after'); +} else { + // execve() needs the interpreter path because it does not search PATH. + const { child } = spawnSyncAndAssert(process.env.PYTHON || 'python3', [ + '-c', 'import sys; print(sys.executable)', + ], {}); + const python = child.stdout.toString().trim(); + const timeout = common.platformTimeout(10_000) / 1000; + + function runCase(kind, mode = 'exec', mask = 0, alias = false) { + const options = JSON.stringify({ kind, mode, mask, alias, timeout }); + spawnSyncAndAssert(python, [fixture, 'run', process.execPath, __filename, options], { + stdout(output) { + const { status, flags, snapshots } = JSON.parse(output); + assert.strictEqual(status, 0); + if (mode === 'exec') { + assert.deepStrictEqual(flags, { nonblocking: mask, closeOnExec: 0, append: 0 }); + } else { + assert.deepStrictEqual(snapshots.map(({ phase }) => phase), ['before', 'setter', 'after']); + assert.strictEqual(snapshots[0].nonblocking, 7); + for (const snapshot of snapshots.slice(1)) { + assert.deepStrictEqual(snapshot.flags, snapshots[0].flags, snapshot.phase); + } + } + }, + }); + } + + for (const kind of ['pipe', 'socket']) { + for (const mask of [0, 1, 2, 4, 7]) { + runCase(kind, 'exec', mask); + } + for (const alias of [false, true]) { + if (alias) { + runCase(kind, 'exec', 0, alias); + runCase(kind, 'exec', 7, alias); + } + runCase(kind, 'rollback', 0, alias); + runCase(kind, 'closed', 0, alias); + } + } + + // A descriptor redirected to a different file keeps its own status flags. + tmpdir.refresh(); + const output = tmpdir.resolve('execve-stdio'); + writeFileSync(output, ''); + spawnSyncAndAssert(process.execPath, ['--no-warnings', __filename, 'reopened', python, output], {}); + assert.deepStrictEqual(JSON.parse(readFileSync(output, 'utf8')), { + nonblocking: 2, closeOnExec: 0, append: 2, + }); +}