Skip to content

fix: InflightManager::enqueue leaks a duplicate close flag to the leader - #1310

Open
madesroches wants to merge 1 commit into
foyer-rs:mainfrom
madesroches:fix/inflight-enqueue-shares-close-flag
Open

fix: InflightManager::enqueue leaks a duplicate close flag to the leader#1310
madesroches wants to merge 1 commit into
foyer-rs:mainfrom
madesroches:fix/inflight-enqueue-shares-close-flag

Conversation

@madesroches

Copy link
Copy Markdown

The Entry::Vacant arm created two separate Arc close flags: one stored in the inflight table, another handed to the fetch leader. take() (called by insert()/cancellation) only flips the table's copy, so the leader's RawFetch::poll -- which reads its own copy -- never observes cancellation. A racing insert() of fresh bytes can be clobbered by the stale leader's re-insert of the value it already had in flight, since the leader is a detached spawned task that outlives any caller scope.

Share a single Arc between the table entry and the leader instead.

The Entry::Vacant arm created two separate Arc<AtomicBool> close flags: one
stored in the inflight table, another handed to the fetch leader. take()
(called by insert()/cancellation) only flips the table's copy, so the
leader's RawFetch::poll -- which reads its own copy -- never observes
cancellation. A racing insert() of fresh bytes can be clobbered by the
stale leader's re-insert of the value it already had in flight, since the
leader is a detached spawned task that outlives any caller scope.

Share a single Arc between the table entry and the leader instead.
@madesroches

Copy link
Copy Markdown
Author

Apologies — I didn't spot this before opening, but this fix is the same as #1289, which was there first (opened months ago). The core change to the Entry::Vacant arm in inflight.rs (hoisting the close Arc and cloning it into the table entry so the leader and table share one flag) is identical. Credit for the fix belongs to #1289.

The only extra here is a small unit test in inflight.rs asserting the Lead close flag observes take() through the shared Arc, which complements #1289's end-to-end test in raw.rs. Happy to move that test over to #1289 and close this out.

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
foyer-memory/src/inflight.rs 92.30% 1 Missing ⚠️
Files with missing lines Coverage Δ
foyer-memory/src/inflight.rs 90.90% <92.30%> (+0.82%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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