Skip to content

fix(search): bound grep and rg output memory - #3849

Open
jazelly wants to merge 1 commit into
rtk-ai:developfrom
jazelly:fix/rg-bounded-memory
Open

jazelly wants to merge 1 commit into
rtk-ai:developfrom
jazelly:fix/rg-bounded-memory

Conversation

@jazelly

@jazelly jazelly commented Sep 3, 2026 •

Copy link
Copy Markdown

Summary

  • Bounds RTK memory usage when rg or grep produces very large output.
  • Preserves the existing grouped search and tee behavior.
  • Provides an explicit rtk proxy <engine> ... escape hatch whenever captured output is capped.

Root cause

  • The grouped search path reached Command::output() through exec_capture_stdin, causing RTK to retain complete stdout and stderr before compression.
  • RTK then cloned and reformatted the captured output, allowing memory usage to grow far beyond the native search process.

Implementation

  • Drains search stdout and stderr concurrently while retaining at most 10 MiB from each stream.
  • Sends native format and shape output directly through inherited streams instead of buffering it inside RTK.
  • Caps individual streamed lines and retained filtered output at 10 MiB.
  • Uses sync_channel(1) to prevent producer backlog from growing without bound.
  • Terminates and reaps wrapped processes after broken pipes or capture-reader failures.

Rationale

  • The patch changes the capture and lifecycle boundaries instead of replacing the existing search grouping algorithm.
  • The bounded buffers prevent memory growth while keeping the established filtering and tee output contracts intact.
  • RSS assertions remain outside CI because allocator and operating-system measurements are environment-dependent.

Regression coverage

  • Exercises a real rg search whose matching line exceeds 10 MiB and verifies the recovery hint.
  • Verifies that a piped grep exits promptly after its downstream stdout consumer closes.
  • Retains the existing search formatting, error propagation, grouping, and passthrough suites.

Validation

  • cargo fmt --all --check passes.
  • cargo clippy --all-targets --all-features -- -D warnings passes.
  • cargo test --all passes with 3,045 unit tests, zero failures, eight ignored tests, and all integration suites passing.
  • Independent review approved the minimized implementation after verifying bounded memory and child-process lifecycle behavior.

Memory benchmarks

  • A 256 MiB all-match workload reduced peak RSS from 2,336,784,384 bytes to 64,290,816 bytes, or about 97.2 percent.
  • A 128 MiB stderr-only workload peaked at 33,832,960 bytes.
  • A single 128 MiB match line peaked at 95,371,264 bytes.

Issue

@CLAassistant

CLAassistant commented Sep 3, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rtk rg can consume over 50 GB by buffering and duplicating large search output

2 participants