process: throw on execve(2) failure instead of aborting

When the underlying execve(2) system call fails, process.execve()
previously printed an error to stderr and called ABORT(), preventing
JS code from detecting or recovering from common failures such as a
missing binary. Throw an ErrnoException instead, carrying the standard
code, errno, syscall, and path properties.

To leave the process in a clean state when execve(2) fails, no longer
run native AtExit callbacks before the call (their in-memory effects
are discarded on success anyway), and snapshot and restore the
FD_CLOEXEC flags on stdio so a failed call has no observable side
effects. Rename and update test-process-execve-abort.js accordingly
and document the new behavior.

Signed-off-by: Bryan English <[email protected]>
PR-URL: #62878
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>

Author: bengl

doc/api/process.md

@@ -1735,6 +1735,13 @@ that started the Node.js process. Symbolic links, if any, are resolved.
 added:
   - v23.11.0
   - v22.15.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/62878
+    description: A failed `execve(2)` system call now throws an exception
+                 instead of aborting the process. Native `AtExit`
+                 callbacks registered via the embedder API are no longer
+                 invoked before the `execve(2)` call.
 -->
 
 > Stability: 1 - Experimental
@@ -1751,10 +1758,19 @@ 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.
 
-All other resources are discarded by the system when the processes are swapped, without triggering
-any exit or close events and without running any cleanup handler.
-
-This function will never return, unless an error occurred.
+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')`),
+and without invoking native `AtExit` callbacks registered through the
+embedder API. Callers that need to run cleanup logic should do so before
+calling `process.execve()`.
+
+This function does not return on success. If the underlying `execve(2)`
+system call fails, an `Error` is thrown whose `code` property is set to the
+corresponding `errno` string (for example, `'ENOENT'` when `file` does not
+exist), with `syscall` set to `'execve'` and `path` set to `file`. When
+`execve(2)` fails the current process continues to run with its state
+unchanged, so a caller may handle the error and take another action.
 
 This function is not available on Windows or IBM i.
 

src/node_process_methods.cc

@@ -510,15 +510,22 @@ static void ReallyExit(const FunctionCallbackInfo<Value>& 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;
   }
 
-  flags &= ~FD_CLOEXEC;
-  return fcntl(fd, F_SETFD, flags);
+  if (fcntl(fd, F_SETFD, flags & ~FD_CLOEXEC) < 0) {
+    return -1;
+  }
+
+  return flags;
 }
 
 static void Execve(const FunctionCallbackInfo<Value>& args) {
@@ -571,32 +578,41 @@ static void Execve(const FunctionCallbackInfo<Value>& args) {
 
   envp[envp_array->Length()] = nullptr;
 
-  // Set stdin, stdout and stderr to be non-close-on-exec
-  // so that the new process will inherit it.
-  if (persist_standard_stream(0) < 0 || persist_standard_stream(1) < 0 ||
-      persist_standard_stream(2) < 0) {
-    env->ThrowErrnoException(errno, "fcntl");
-    return;
+  // 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};
+  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");
+      return;
+    }
+    saved_stdio_flags[fd] = prev;
   }
 
   // Perform the execve operation.
-  RunAtExit(env);
+  //
+  // 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());
 
-  // If it returns, it means that the execve operation failed.
-  // In that case we abort the process.
-  auto error_message = std::string("process.execve failed with error code ") +
-                       errors::errno_string(errno);
-
-  // Abort the process
-  Local<v8::Value> exception =
-      ErrnoException(isolate, errno, "execve", *executable);
-  Local<v8::Message> message = v8::Exception::CreateMessage(isolate, exception);
-
-  std::string info = FormatErrorMessage(
-      isolate, context, error_message.c_str(), message, true);
-  FPrintF(stderr, "%s\n", info);
-  ABORT();
+  // 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]);
+  }
+  env->ThrowErrnoException(execve_errno, "execve", nullptr, *executable);
 }
 #endif
 

test/parallel/test-process-execve-abort.js

@@ -0,0 +1,39 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { isMainThread } = require('worker_threads');
+
+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');
+}
+
+assert.throws(
+  () => {
+    process.execve(
+      `${process.execPath}_non_existing`,
+      [process.execPath, 'arg'],
+    );
+  },
+  (err) => {
+    assert.ok(err instanceof Error);
+    assert.strictEqual(err.code, 'ENOENT');
+    assert.strictEqual(err.syscall, 'execve');
+    assert.strictEqual(typeof err.errno, 'number');
+    assert.ok(err.errno > 0, `expected positive errno, got ${err.errno}`);
+    assert.match(err.path, /_non_existing$/);
+    return true;
+  },
+);
+
+assert.strictEqual(process.stdout.writable, true);
+assert.strictEqual(process.stderr.writable, true);
+assert.strictEqual(typeof process.pid, 'number');
+
+let tickFired = false;
+process.nextTick(common.mustCall(() => { tickFired = true; }));
+setImmediate(common.mustCall(() => {
+  assert.strictEqual(tickFired, true);
+}));