fix: cancel the pending slot state timeout when a pool slot is shut down - #1283
Open
pjfanning wants to merge 1 commit into
Open
fix: cancel the pending slot state timeout when a pool slot is shut down#1283pjfanning wants to merge 1 commit into
pjfanning wants to merge 1 commit into
Conversation
Motivation: Every state transition in `NewHostConnectionPool`'s slot state machine cancels the timeout of the state it leaves and, if the new state defines one, schedules a fresh `materializer.scheduleOnce` task for it. Those tasks are not bound to the stage lifecycle, so they keep running after the stage has stopped. `Slot.shutdown()`, which `postStop` calls for every slot, closed the connection and ran `state.onShutdown` but never cancelled that timeout. A pool that stops while a slot is in a state with a finite timeout - waiting for a connection, for a response entity subscription, or for the keep-alive timeout - therefore leaves a scheduled task behind that keeps the slot, and through it the whole pool logic with its connections and request contexts, reachable until the timeout elapses. Firing does nothing useful either: the task only invokes an async callback on a stage that is already gone. Modification: Cancel the current timeout at the start of `Slot.shutdown()`, like every state transition and the slot error path already do. Result: Shutting a pool down no longer leaves scheduled state timeouts, and the pool becomes collectable as soon as it stops. Tests: - sbt "http-core/testOnly org.apache.pekko.http.impl.engine.client.pool.NewHostConnectionPoolSpec" - pass (1 test). Reverting only the `cancelCurrentTimeout()` call makes it fail, as the scheduled task is still live after the pool stopped. - sbt "http-core/mimaReportBinaryIssues" - pass. - scalafmt --mode diff-ref=upstream/main --test - pass. References: None - found while auditing `src/main` for resource leaks
pjfanning
marked this pull request as ready for review
September 7, 2026 21:04
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
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.
Motivation
Every state transition in
NewHostConnectionPool's slot state machine cancels the timeout of the state it leaves and, if the new state defines one, schedules a freshmaterializer.scheduleOncetask for it. Those tasks are not bound to the stage lifecycle, so they keep running after the stage has stopped.Slot.shutdown(), whichpostStopcalls for every slot, closed the connection and ranstate.onShutdownbut never cancelled that timeout. A pool that stops while a slot is in a state with a finite timeout — waiting for a connection, for a response entity subscription, or for the keep-alive timeout — therefore leaves a scheduled task behind that keeps the slot, and through it the whole pool logic with its connections and request contexts, reachable until the timeout elapses. Firing does nothing useful either: the task only invokes an async callback on a stage that is already gone.Modification
Cancel the current timeout at the start of
Slot.shutdown(), like every state transition and the slot error path already do.Result
Shutting a pool down no longer leaves scheduled state timeouts, and the pool becomes collectable as soon as it stops.
Tests
NewHostConnectionPoolSpec: the pool flow is driven with probes until a slot waits for a response entity subscription that never comes, then the pool is torn down. A test scheduler records the tasks scheduled with that state's (distinctive) timeout so the test can assert they end up cancelled. Reverting only thecancelCurrentTimeout()call makes it fail — the task is still live after the pool stopped.sbt "http-core/testOnly org.apache.pekko.http.impl.engine.client.pool.NewHostConnectionPoolSpec"— pass (1 test).sbt "http-core/mimaReportBinaryIssues"— pass.scalafmt --mode diff-ref=upstream/main --test— pass.References
None — found while auditing
src/mainfor resource leaks.