Skip to content

fix(runtime): saturate FlushingPolicyState counters to prevent add-overflow panic (#1359)#1413

Open
teddytennant wants to merge 1 commit into
tracel-ai:mainfrom
teddytennant:fix/flushing-policy-saturate-1359
Open

fix(runtime): saturate FlushingPolicyState counters to prevent add-overflow panic (#1359)#1413
teddytennant wants to merge 1 commit into
tracel-ai:mainfrom
teddytennant:fix/flushing-policy-saturate-1359

Conversation

@teddytennant

Copy link
Copy Markdown

What

FlushingPolicyState::register now accumulates its bytes_count and bytes_size counters with saturating_add instead of +=, and converts the allocation length with u32::try_from(bytes.len()).unwrap_or(u32::MAX) instead of as u32.

Why

PendingDropQueue::push() calls register() on every staged Bytes allocation, growing the two u32 counters. Flushing is advisory — the consumer decides when to call flush()/reset() based on should_flush() — so between flushes the counters can grow without bound. On a large workload (reported in #1359 while training a resnet model on a 24 GB GPU; more data-loader workers reproduce it faster) self.bytes_size += bytes.len() as u32 eventually exceeds u32::MAX:

  • in debug (overflow-checks on) it panics with attempt to add with overflow at policy.rs, the site reported in the issue;
  • in release it silently wraps, corrupting the flush decision.

saturating_add can never panic or wrap. Because should_flush() compares with >=, saturating at u32::MAX never changes the flush decision — once at/over threshold a flush+reset follows. The saturating usize -> u32 conversion also fixes a latent truncation bug: a single allocation >= 4 GiB previously truncated to a small wrong value and could suppress a needed flush; it now counts as u32::MAX, guaranteeing a flush.

No public API change (the u32 fields on FlushingPolicy are untouched), the default max_bytes_size of 64 MiB means normal configs are unaffected, and no-std/alloc compatibility is preserved.

Testing

Added register_saturates_instead_of_overflowing to the existing policy_tests module. It stages an 8-byte allocation on a state whose counters are already at/near u32::MAX. On the current code this panics with attempt to add with overflow (debug overflow-checks); with the fix the counters saturate and should_flush returns true.

cargo test -p cubecl-runtime -- memory_management::drop_queue::policy

All 6 policy tests pass. cargo fmt --check and cargo clippy -p cubecl-runtime --all-targets -- -D warnings are clean.

Fixes #1359

…erflow panic (tracel-ai#1359)

`FlushingPolicyState::register` accumulated its two u32 counters with `+=`
on every staged `Bytes` allocation. Flushing is advisory, so between the
consumer's flush/reset calls the counters can grow without bound; on a large
workload `bytes_size += bytes.len() as u32` eventually exceeds `u32::MAX` and
panics with "attempt to add with overflow" (release builds silently wrap and
corrupt the flush decision).

Use `saturating_add` for both counters, and convert the length with
`u32::try_from(..).unwrap_or(u32::MAX)` so a single >= 4 GiB allocation
saturates instead of truncating. Because `should_flush` compares with `>=`,
saturating at `u32::MAX` never changes the flush decision. No public API
change.

Fixes tracel-ai#1359
Copilot AI review requested due to automatic review settings July 8, 2026 21:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

Hitting addition with overflow in FlushingPolicyState

2 participants