fix: cancel open request timeouts when the server connection stage stops - #1284
Open
pjfanning wants to merge 1 commit into
Open
fix: cancel open request timeouts when the server connection stage stops#1284pjfanning wants to merge 1 commit into
pjfanning wants to merge 1 commit into
Conversation
Motivation: `RequestTimeoutSupport` schedules a request's timeout through `materializer.scheduleOnce` and keeps the `TimeoutAccess` in `openTimeouts` until the response for that request comes back, which is when the timeout is cancelled. The stage had no `postStop`, so a connection that goes away with requests still open - the client disconnecting, the connection being aborted, or the server being torn down - left one scheduled task per open request behind. Those tasks keep the request, the timeout handler and the stage's async callback reachable for as long as the configured `request-timeout` (20 seconds by default), and firing them achieves nothing: they only invoke a callback on a stage that is already gone. Modification: Cancel everything still in `openTimeouts` in `postStop`, using the same best-effort `clear()` the regular response path uses. Result: Connections that end before their requests are answered no longer leave scheduled timeouts behind. Tests: - sbt "http-core/testOnly org.apache.pekko.http.impl.engine.server.HttpServerSpec org.apache.pekko.http.impl.engine.server.HttpServerWithExplicitSchedulerSpec" - pass (66 tests). Reverting only the `postStop` addition makes the new test fail, as the timeout still fires after the stage 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:03
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
RequestTimeoutSupportschedules a request's timeout throughmaterializer.scheduleOnceand keeps theTimeoutAccessinopenTimeoutsuntil the response for that request comes back, which is when the timeout is cancelled.The stage had no
postStop, so a connection that goes away with requests still open — the client disconnecting, the connection being aborted, or the server being torn down — left one scheduled task per open request behind. Those tasks keep the request, the timeout handler and the stage's async callback reachable for as long as the configuredrequest-timeout(20 seconds by default), and firing them achieves nothing: they only invoke a callback on a stage that is already gone.Modification
Cancel everything still in
openTimeoutsinpostStop, using the same best-effortclear()the regular response path uses.Result
Connections that end before their requests are answered no longer leave scheduled timeouts behind.
Tests
HttpServerWithExplicitSchedulerSpec:RequestTimeoutSupportis driven with probes, a request gets a timeout handler that records being called, and both sides are then completed with the request still unanswered. With the explicitly triggered scheduler the test can then let the timeout elapse and assert the handler never runs. Reverting only thepostStopaddition makes it fail.sbt "http-core/testOnly org.apache.pekko.http.impl.engine.server.HttpServerSpec org.apache.pekko.http.impl.engine.server.HttpServerWithExplicitSchedulerSpec"— pass (66 tests), including the five existing request-timeout tests.sbt "http-core/mimaReportBinaryIssues"— pass.scalafmt --mode diff-ref=upstream/main --test— pass.References
None — found while auditing
src/mainfor resource leaks.