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

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-throws.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);
+}));