fix: close correct pipe fds in _open_pipes error cleanup - #286
Draft
toddr-bot wants to merge 1 commit into
Draft
Conversation
_open_pipes() pushed $op->{KFD} (the child's target fd number, e.g. 0
for stdin) instead of $op->{TFD} (the actual pipe read-end fd) into
@close_on_fail for SCALAR and CODE input redirects. On error,
_close($op->{KFD}) would close fd 0 (STDIN) in the parent process.
Also add the read end of inter-child pipes to @close_on_fail so it
doesn't leak when a later operation fails in the same _open_pipes call.
The output redirect paths ('>') already used the correct $op->{FD} and
$op->{TFD}; only the input redirect paths ('<') had the bug.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes fd leak and potential STDIN corruption in
_open_pipes()error cleanup path.Why
For SCALAR and CODE input redirects,
@close_on_failreceived$op->{KFD}(the child's target fd number, typically 0 for stdin) instead of$op->{TFD}(the actual pipe read-end fd). When a later operation failed and error cleanup ran,_close(0)would close STDIN in the parent process.The output redirect paths (
>) already used the correct$op->{FD}and$op->{TFD}— only the input paths (<) had the bug.Also adds the read end of inter-child pipes (
$pipe_read_fd) to@close_on_failto prevent a leak when a later operation in the same_open_pipes()call fails.How
$op->{KFD}to$op->{TFD}inpush @close_on_failfor SCALAR and CODE input casespush @close_on_fail, $pipe_read_fdafter inter-child pipe creationTesting
Added 4 tests in
t/run.tusing_simulate_open_failureto trigger the error path after SCALAR/CODE input pipes are created. Verified_map_fdsis unchanged after cleanup (would detect a leaked or wrongly-closed fd).🤖 Generated with Claude Code
Quality Report
Changes: 2 files changed, 20 insertions(+), 3 deletions(-)
Code scan: clean
Tests: failed (0 Failed, 37 tests)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline