Skip to content

fix: cancel the pending slot state timeout when a pool slot is shut down - #1283

Open
pjfanning wants to merge 1 commit into
apache:mainfrom
pjfanning:pool-slot-cancel-timeout-on-shutdown
Open

fix: cancel the pending slot state timeout when a pool slot is shut down#1283
pjfanning wants to merge 1 commit into
apache:mainfrom
pjfanning:pool-slot-cancel-timeout-on-shutdown

Conversation

@pjfanning

Copy link
Copy Markdown
Member

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

  • New 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 the cancelCurrentTimeout() 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/main for resource leaks.

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
pjfanning marked this pull request as ready for review September 7, 2026 21:04
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