Skip to content

child_process: watch child_process stdin pipe peer close event - #62353

Open
Tseian wants to merge 1 commit into
nodejs:mainfrom
Tseian:main
Open

child_process: watch child_process stdin pipe peer close event#62353
Tseian wants to merge 1 commit into
nodejs:mainfrom
Tseian:main

Conversation

@Tseian

@Tseian Tseian commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Issue

#25131

Description

Watch pipe peer close(EOF/HUP) event, only support for unix. Once the event is detected, a JS callback is triggered to execute, which in turn triggers a method to destroy the socket. Eventually child_process.stdin.on('close') will be triggered.

Before

child_process.stdin.on('close') will not be triggered if the readable end of the pipe has been closed.

After

child_process.stdin.on('close') will be triggered if the readable end of the pipe has been closed.

Changes

Test

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Mar 20, 2026
@Tseian Tseian changed the title child-process: watch pipe peer close event WIP child-process: watch pipe peer close event Mar 20, 2026
@Tseian
Tseian force-pushed the main branch 3 times, most recently from 071fa18 to 4cedb07 Compare March 21, 2026 06:11
@codecov

codecov Bot commented Mar 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.11940% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.18%. Comparing base (6f41e41) to head (f484e35).

Files with missing lines Patch % Lines
src/pipe_wrap.cc 73.68% 3 Missing and 12 partials ⚠️
lib/internal/child_process.js 90.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #62353   +/-   ##
=======================================
  Coverage   90.17%   90.18%           
=======================================
  Files         770      770           
  Lines      264483   264547   +64     
  Branches    50262    50284   +22     
=======================================
+ Hits       238491   238571   +80     
+ Misses      16981    16944   -37     
- Partials     9011     9032   +21     
Files with missing lines Coverage Δ
src/pipe_wrap.h 100.00% <ø> (ø)
lib/internal/child_process.js 95.02% <90.00%> (+0.03%) ⬆️
src/pipe_wrap.cc 77.29% <73.68%> (-1.61%) ⬇️

... and 22 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.

@Tseian
Tseian force-pushed the main branch 2 times, most recently from 0ae2360 to 09463c0 Compare March 21, 2026 08:21
@Tseian Tseian changed the title WIP child-process: watch pipe peer close event child-process: watch pipe peer close event Mar 21, 2026
@Tseian
Tseian force-pushed the main branch 2 times, most recently from 9e25a20 to 78acd67 Compare March 21, 2026 14:26
@Tseian Tseian changed the title child-process: watch pipe peer close event child-process: watch child_process stdin pipe peer close event Mar 21, 2026
@Tseian
Tseian force-pushed the main branch 3 times, most recently from e704d96 to 8cc7608 Compare March 22, 2026 01:43
@Tseian Tseian changed the title child-process: watch child_process stdin pipe peer close event child_process: watch child_process stdin pipe peer close event Mar 22, 2026
@Tseian

Tseian commented Mar 22, 2026

Copy link
Copy Markdown
Contributor Author

@nodejs-github-bot retest

@Tseian

Tseian commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

@jasnell @ronag @jbunton-atlassian Hi everyone, Could you please have a code review for this PR?

Comment thread src/pipe_wrap.cc
Comment thread src/pipe_wrap.cc Outdated
Comment thread src/pipe_wrap.cc Outdated
@Tseian

Tseian commented Mar 24, 2026

Copy link
Copy Markdown
Contributor Author

@jasnell Please have a code review again for this PR.

  • Deleted wnwatchPeerClose method
  • watchPeerClose no longer returns values

@addaleax addaleax added child_process Issues and PRs related to the child_process subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Mar 24, 2026
Comment thread src/pipe_wrap.cc Outdated
Comment thread src/pipe_wrap.h Outdated
Comment thread src/pipe_wrap.cc Outdated
Comment thread src/pipe_wrap.cc Outdated
Comment thread lib/internal/child_process.js
@Tseian
Tseian force-pushed the main branch 2 times, most recently from 574f851 to 1a34143 Compare March 25, 2026 04:13
Watch the child end of the stdin pipe for peer-close (EOF) events, only
supported on non-Windows platforms. When the child closes its end of the
pipe, subprocess.stdin is destroyed so that its 'close' event is emitted,
instead of only being reported as an EPIPE error on the next write.

Fixes: nodejs#25131
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++. child_process Issues and PRs related to the child_process subsystem. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants