Skip to content

fix(core): avoid deadlock on here-documents larger than the pipe buffer - #1293

Open
cataggar wants to merge 1 commit into
reubeno:mainfrom
cataggar:fix/heredoc-pipe-deadlock
Open

cataggar wants to merge 1 commit into
reubeno:mainfrom
cataggar:fix/heredoc-pipe-deadlock

Conversation

@cataggar

Copy link
Copy Markdown
Contributor

Overview

Fix a deadlock when a here-document or here-string is larger than the pipe buffer.

This change was made by GitHub Copilot at my direction.

Details

setup_open_file_with_contents currently writes the complete payload into an anonymous pipe before the command that reads from it has been spawned. The write blocks indefinitely when the payload exceeds the available pipe capacity. This is especially visible on Windows, where the default pipe buffer is 4 KiB.

This change:

  • keeps direct writes for payloads up to 4 KiB;
  • retains Linux and Android pipe resizing as a best-effort fast path;
  • falls back to a named writer thread when the payload cannot safely fit;
  • avoids failing redirection when F_SETPIPE_SZ rejects an empty payload or one above the system pipe-size limit; and
  • adds bash-oracle compatibility cases for large here-documents and here-strings.

The helper thread exits after the reader consumes the payload, or earlier on a broken pipe if the command closes the read end without consuming it.

Testing

  • cargo test --test brush-compat-tests -- here
  • cargo xtask check fmt
  • cargo clippy --package brush-core --package brush-shell --all-features --all-targets

`setup_open_file_with_contents` creates an anonymous pipe and writes the
entire here-document / here-string payload into it before the reading
command exists. That write can only complete while the payload still fits
in the pipe buffer, so any larger payload blocks forever.

Linux papered over this by growing the buffer to the payload size with
F_SETPIPE_SZ. Every other platform kept the default buffer -- 4 KiB on
Windows -- so a here-document past that threshold hangs the shell. The
Linux path had gaps of its own: the fcntl is propagated with `?`, so it
also failed outright for payloads over /proc/sys/fs/pipe-max-size (1 MiB
by default for unprivileged processes) and for a zero-length payload.

Keep the resize as a best-effort fast path and hand oversized payloads to
a helper thread instead, so the write proceeds while the reader drains.
The thread ends when the payload is consumed, or earlier with a broken
pipe if the read end is dropped by a command that never reads its stdin.

Found running an 88k-line bash CLI under brush on Windows: every
`cat <<EOF`-based help function hung.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 17.42 μs 17.40 μs -0.02 μs ⚪ Unchanged
eval_arithmetic 0.15 μs 0.15 μs 0.00 μs ⚪ Unchanged
expand_one_string 1.60 μs 1.73 μs 0.13 μs 🟠 +8.26%
for_loop 31.05 μs 31.09 μs 0.04 μs ⚪ Unchanged
full_peg_complex 59.88 μs 58.58 μs -1.30 μs 🟢 -2.17%
full_peg_for_loop 6.42 μs 6.23 μs -0.19 μs 🟢 -2.96%
full_peg_nested_expansions 16.42 μs 16.36 μs -0.06 μs ⚪ Unchanged
full_peg_pipeline 4.53 μs 4.38 μs -0.15 μs 🟢 -3.38%
full_peg_simple 1.92 μs 1.86 μs -0.06 μs 🟢 -3.03%
function_call 3.34 μs 3.35 μs 0.01 μs ⚪ Unchanged
instantiate_shell 53.76 μs 54.18 μs 0.42 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 25963.56 μs 26866.98 μs 903.42 μs 🟠 +3.48%
parse_peg_bash_completion 2137.46 μs 2120.36 μs -17.10 μs 🟢 -0.80%
parse_peg_complex 21.86 μs 21.42 μs -0.44 μs ⚪ Unchanged
parse_peg_for_loop 2.16 μs 2.08 μs -0.08 μs 🟢 -3.66%
parse_peg_pipeline 2.31 μs 2.32 μs 0.01 μs ⚪ Unchanged
parse_peg_simple 1.22 μs 1.17 μs -0.04 μs 🟢 -3.45%
run_echo_builtin_command 16.40 μs 16.12 μs -0.28 μs ⚪ Unchanged
tokenize_sample_script 3.46 μs 3.42 μs -0.04 μs ⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
Overall Coverage 🟢 27.49% 🟢 27.48% 🔴 -0.01%

Minimum allowed coverage is 20%, this run produced 27.48%
Maximum allowed coverage difference is -5%, this run produced -0.01%

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1583 75.06
❗️ Error 17 0.81
❌ Fail 155 7.35
⏩ Skip 339 16.07
❎ Expected Fail 13 0.62
✔️ Unexpected Pass 2 0.09
📊 Total 2109 100.00

@reubeno reubeno added the state: needs review PR or issue author is waiting for a review label Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state: needs review PR or issue author is waiting for a review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants