Skip to content

Release owned queue handles after grant completion - #131

Open
cf-rhett wants to merge 2 commits into
jamesmunns:mainfrom
cf-rhett:fix-owned-grant-handle-leak
Open

Release owned queue handles after grant completion#131
cf-rhett wants to merge 2 commits into
jamesmunns:mainfrom
cf-rhett:fix-owned-grant-handle-leak

Conversation

@cf-rhett

Copy link
Copy Markdown

Owned framed and stream grants call mem::forget(self) after commit or release to suppress their default Drop behavior. For ArcBBQueue, that also permanently retains each cloned Arc and can keep the queue allocation alive indefinitely.

Stream grants now record the requested terminal amount and use their existing Drop path for coordination. Framed grants suppress their default Drop only after coordination succeeds, then notify through and release the extracted queue handle. Public APIs and grant layouts are unchanged.

This surfaced while investigating #130.

Miri evidence

On pristine f212a93, removing -Zmiri-ignore-leaks makes the pre-existing test::arc2 report the leaked 768-byte BBQueue allocation from ArcBBQueue::new_with_storage and the leaked 64-byte ring allocation from BoxedSlice::new. The retained queue also roots notifier, waker, and Tokio task state, producing 21 leaked allocations in total.

This branch removes that suppression. The exact CI entry point, ./miri.sh, passes all 14 unit tests and 2 doctests with leak checking enabled and no leak reports.

Validation

  • ./miri.sh
  • cargo test -p bbqueue --features std
  • cargo clippy -p bbqueue --all-targets --features std -- -D warnings
  • default and no-default-features builds

cf-rhett added 2 commits July 30, 2026 18:09
Terminal grant methods forgot the entire grant to bypass its default Drop behavior. With ArcBBQueue, that retained an Arc after every commit or release and kept the queue allocation alive indefinitely.

Release the owned handle while preserving terminal coordination, and cover each affected method with an Arc liveness regression test.
The existing Arc-backed tests expose owned handle leaks, but the Miri
script disabled leak reports. Keep the checker enabled now that the
suite is leak-free so CI catches future regressions.
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