Repository navigation
Fix pool slot leak on cancelled checkout that wedges the pool (#20) - #22
Merged
Merged
Conversation
Pool mode: `Pool::checkout` reserves a slot (`bucket.total += 1`) under the lock, then awaits `create_connection`. It runs inside the per-connection handshake timeout (`tokio::time::timeout`), so when the upstream accepts the TCP connection but is slow or unresponsive to the startup, that await can be dropped mid-flight. Neither the Ok nor the Err arm then runs, and the `PoolLease` guard is only created after checkout returns, so the reserved slot was never decremented. Each cancelled checkout leaked one phantom slot; once `total` reached `pool_size` with no real connections, every later checkout found the bucket "full" with nothing to hand out and failed `pool checkout timeout: all connections in use` — a permanent wedge cleared only by restart. Pre-existing since pooling (0.3); present in 1.0.0–1.0.3. Not a regression from the #11 work — 1.0.2 (0ce3be6) wedges identically under the same probe. Fix: hold the reserved slot in an RAII guard (`SlotReservation`) that decrements `total` on every drop — Err return or cancellation — and is disarmed only once the connection exists and its slot is owned by the returned conn. Mirrors the `PoolLease` guard from #11. Regression coverage (Suite 9, tests/run.sh): a black-hole upstream (tests/blackhole.mjs) that accepts TCP but never responds forces every create_connection to be cancelled; the test asserts the bucket total returns to 0 and the pool is not wedged. Verified red on 1.0.2/1.0.3 (total pinned at pool_size, "all connections in use") and green after (total 0, fresh checkout still attempts a real connect). Deterministic black-hole probe confirms the fix holds `total` at 0 across repeated cancellations. Bump version to 1.0.4.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #20. Hotfix release 1.0.4 (#20 only; #15/#14 follow as 1.0.5).
The defect
In pool mode,
Pool::checkoutreserves a slot (bucket.total += 1) before awaitingcreate_connection. That runs inside the per-connection handshake timeout, so when the upstream accepts the socket but is slow to answer the startup, the future is dropped mid-await and neither the Ok nor Err arm runs — the reserved slot is never released, and thePoolLeaseguard doesn't exist yet. Each cancelled checkout leaks one phantom slot; oncetotalreachespool_sizewith no real connections, every later checkout failspool checkout timeout: all connections in use— a permanent wedge cleared only by restart.Pre-existing since pooling (0.3); present in 1.0.0–1.0.3. Not a regression from #11 — 1.0.2 (
0ce3be6) wedges identically under the same probe.The fix
An RAII
SlotReservationguard holds the reserved slot and decrementstotalon every exit — Err return or cancellation — and is disarmed only once the connection exists and owns the slot. Mirrors thePoolLeaseguard from #11.Verification
cargo fmt --check,cargo clippy -- -D warnings,cargo test(94 unit tests) — green../tests/run.sh52/52, including new Suite 9: a black-hole upstream (tests/blackhole.mjs) that forces everycreate_connectionto be cancelled, asserting the bucket total returns to 0 and the pool is not wedged.totalatpool_sizeand report "all connections in use"; fixed returnstotalto 0 and a fresh checkout still attempts a real connect.Bumps version to 1.0.4.