Skip to content

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

Open
bengl wants to merge 1 commit intonodejs:mainfrom
bengl:bengl/execve-throws
Open

process: throw on execve(2) failure instead of aborting#62878
bengl wants to merge 1 commit intonodejs:mainfrom
bengl:bengl/execve-throws

Conversation

@bengl
Copy link
Copy Markdown
Member

@bengl bengl commented Apr 21, 2026

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 usual 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 behaviour.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Apr 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 52.63158% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.62%. Comparing base (c29a34c) to head (55e4b8f).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/node_process_methods.cc 52.63% 7 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62878      +/-   ##
==========================================
- Coverage   89.62%   89.62%   -0.01%     
==========================================
  Files         706      706              
  Lines      219165   219170       +5     
  Branches    41989    42000      +11     
==========================================
+ Hits       196435   196439       +4     
+ Misses      14621    14615       -6     
- Partials     8109     8116       +7     
Files with missing lines Coverage Δ
src/node_process_methods.cc 88.52% <52.63%> (+0.34%) ⬆️

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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 <bryan@bryanenglish.com>
@bengl bengl force-pushed the bengl/execve-throws branch from 55e9403 to 55e4b8f Compare April 22, 2026 00:17
@bengl bengl added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 22, 2026
@bengl bengl requested a review from ShogunPanda April 22, 2026 00:39
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 22, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@bengl bengl requested a review from jasnell April 22, 2026 13:30
@bengl
Copy link
Copy Markdown
Member Author

bengl commented Apr 22, 2026

@jasnell adding you as a reviewer since you had the initial suggestion to just abort when execve fails. The approach I took here is a bit of a departure, since RunAtExit is no longer called. I think this approach makes sense because the process isn't so much "exiting" as being replaced entirely, as is the nature of execve. The benefit of being able to recover from ENOENT, for example, seems worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants