Skip to content

Cap CollectString/CollectStreams so print loops can't OOM the host#558

Open
Shaurya-Sethi wants to merge 3 commits into
pydantic:mainfrom
Shaurya-Sethi:fix/print-collect-cap-464
Open

Cap CollectString/CollectStreams so print loops can't OOM the host#558
Shaurya-Sethi wants to merge 3 commits into
pydantic:mainfrom
Shaurya-Sethi:fix/print-collect-cap-464

Conversation

@Shaurya-Sethi

@Shaurya-Sethi Shaurya-Sethi commented Jul 10, 2026

Copy link
Copy Markdown

Summary

  • Cap host-side CollectString / CollectStreams buffers at 10 MiB by default (max_bytes=None to opt out), since ResourceLimits.max_memory only tracks the Monty heap and does not protect parent-process collectors (PrintWriter::CollectString and PrintWriter::CollectStreams probably need to have a size check #464).
  • Enforce the same check in Rust PrintWriter and Python PrintTarget::write_event (pool host path); exceeding raises MemoryError with the familiar memory limit exceeded: … message.
  • Add regression tests and document the divergence in limitations/print.md.

Test plan

  • cargo test -p monty --test print_collect_limits --test print_writer
  • cargo test -p monty --test repl call_function_captures_print -- --exact
  • make format-rs / make lint-rs
  • uv run pytest crates/monty-python/tests/test_print.py::test_collect_string_max_bytes_raises (and existing collector tests)
  • make lint-py (with CONDA_PREFIX unset if needed)

Summary by cubic

Add a 10 MiB default cap to host-side print collectors (CollectString, CollectStreams) so print loops can’t OOM the host. Python CollectStreams charges a fixed per-entry overhead and tracks bytes in O(1); use max_bytes=None to opt out (issue #464).

  • New Features

    • Default cap: DEFAULT_MAX_PRINT_COLLECT_BYTES (10 MiB). Exceeding raises MemoryError with “memory limit exceeded: {used} bytes > {limit} bytes”. Not covered by ResourceLimits.max_memory.
    • Rust: added check_print_collect_limit; integrated caps into PrintWriter::{CollectString, CollectStreams} (now carry Option<usize>); helpers PrintWriter::collect_string and PrintWriter::collect_streams; exported DEFAULT_MAX_PRINT_COLLECT_BYTES. Tests now cover helper constructors and newline-push limit paths.
    • Python: CollectString(max_bytes=...) and CollectStreams(max_bytes=...) (default 10 MiB), enforced in PrintTarget::write_event. CollectStreams charges a 64-byte per-entry overhead with a running collected_bytes counter. pyi/docs updated; regression tests added.
  • Migration

    • If you constructed enum variants directly, update to PrintWriter::collect_string(&mut buf) or PrintWriter::collect_streams(&mut vec) (or pass an explicit Option<usize> to the variants).
    • To disable the cap: Python CollectString(max_bytes=None) / CollectStreams(max_bytes=None); Rust pass None to the corresponding variant.

Written for commit 9b649cb. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 9 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/monty-python/src/print_target.rs
Comment thread crates/monty-python/src/print_target.rs Outdated
Comment thread crates/monty/tests/print_collect_limits.rs
Comment thread limitations/print.md Outdated
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed-hq

codspeed-hq Bot commented Jul 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 16 skipped benchmarks1


Comparing Shaurya-Sethi:fix/print-collect-cap-464 (9b649cb) with main (c455a30)

Open in CodSpeed

Footnotes

  1. 16 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Charge per-entry overhead toward the host cap, keep a running collected_bytes
counter, tighten the streams MemoryError assertion, and fix the print.md typo.
Codecov flagged uncovered io.rs lines: collect_streams constructor and
append_streams_char / CollectString stdout_push. Prior tests used end=''
so terminators never ran.
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.

1 participant