Skip to content

fix(core): cap adaptive buffer initial allocations - #36432

Merged
bartlomieju merged 1 commit into
denoland:mainfrom
nathanwhit:fix/adaptive-buffer-initial-allocation-cap
Aug 10, 2026
Merged

fix(core): cap adaptive buffer initial allocations#36432
bartlomieju merged 1 commit into
denoland:mainfrom
nathanwhit:fix/adaptive-buffer-initial-allocation-cap

Conversation

@nathanwhit

Copy link
Copy Markdown
Member

Summary

  • use checked power-of-two rounding for adaptive read-buffer starting sizes
  • cap initial allocations at the strategy's existing 16 MiB growth ceiling
  • saturate u64 size hints when the target usize cannot represent them
  • preserve the existing read-driven buffer growth behavior

Details

Adaptive buffer size classes round size hints to a power of two when choosing the initial allocation. Oversized hints could overflow that rounding operation or choose a starting size above the strategy's normal growth ceiling. The initial-size calculation now uses checked rounding and the existing MAX_GROW_LEN cap, while narrower targets convert u64 hints without truncation.

Validation

  • ./tools/format.js libs/core/io/buffer_strategy.rs --check
  • cargo test -p deno_core buffer_strategy -- --nocapture
  • cargo check -p deno_core --all-targets

@bartlomieju bartlomieju left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch, and it's reachable: new_from_hint_u64 is fed size hints that can come straight from a Content-Length, so a hostile Content-Length: 18446744073709551615 reached usize::next_power_of_two(usize::MAX) β€” a panic in debug, and a wrap to 0 in release, which is arguably worse.

The two halves fit together properly: usize::try_from(...).unwrap_or(usize::MAX) stops the as _ truncation on 32-bit targets, and capped_next_power_of_two makes the rounding total. Capping the initial allocation at MAX_GROW_LEN rather than some new constant is the right choice β€” the strategy already refuses to grow past 16 MiB, so starting above it was inconsistent anyway. A genuinely known 32 MiB body now costs one extra realloc, which is a fine trade for not letting a header pick the allocation size.

Test detail worth noting: (usize::MAX as u64).saturating_add(1) works on both 32- and 64-bit β€” it's u64::MAX on 64-bit and 2^32 on 32-bit, and both saturate to usize::MAX through try_from. LGTM.

@bartlomieju
bartlomieju merged commit 0e72c12 into denoland:main Aug 10, 2026
136 checks passed
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.

2 participants