Ensure byte string transformers clean up on every stage stop#1154
Merged
Conversation
Motivation: A byte string transformer marked itself finished before a pending final emission was delivered, so downstream cancellation in that window skipped cleanup. Modification: Treat cleanup as the stage finalizer by invoking it unconditionally from postStop, and add directional tests for normal completion and cancellation while the final emission is pending. Result: All stage termination paths release transformer resources without relying on a fragile finished flag. Existing compressor cleanup remains safe because it is idempotent. Tests: - sbt "http-core / Test / testOnly org.apache.pekko.http.impl.util.StreamUtilsSpec": 7 passed - sbt "http-tests / Test / testOnly org.apache.pekko.http.scaladsl.coding.DeflateSpec org.apache.pekko.http.scaladsl.coding.GzipSpec": 54 passed - sbt checkCodeStyle: passed - sbt headerCreateAll and +headerCheckAll: passed - scalafmt --list --mode diff-ref=origin/main: passed - sbt sortImports: environment failure, Scalafix/scala.meta NoSuchMethodError - sbt validatePullRequest: not run locally per maintainer request; delegated to CI References: Refs apache#1133; follow-up to apache#1142
Member
|
@He-Pin can you back port this to 1.4.x branch? |
He-Pin
added a commit
that referenced
this pull request
Jul 11, 2026
Motivation: A byte string transformer marked itself finished before a pending final emission was delivered, so downstream cancellation in that window skipped cleanup. Modification: Treat cleanup as the stage finalizer by invoking it unconditionally from postStop, and add directional tests for normal completion and cancellation while the final emission is pending. Result: All stage termination paths release transformer resources without relying on a fragile finished flag. Existing compressor cleanup remains safe because it is idempotent. Tests: - sbt "http-core / Test / testOnly org.apache.pekko.http.impl.util.StreamUtilsSpec": 7 passed - sbt "http-tests / Test / testOnly org.apache.pekko.http.scaladsl.coding.DeflateSpec org.apache.pekko.http.scaladsl.coding.GzipSpec": 54 passed - sbt checkCodeStyle: passed - sbt headerCreateAll and +headerCheckAll: passed - scalafmt --list --mode diff-ref=origin/main: passed - sbt sortImports: environment failure, Scalafix/scala.meta NoSuchMethodError - sbt validatePullRequest: not run locally per maintainer request; delegated to CI References: Refs #1133; follow-up to #1142
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
2 participants
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
PR #1142 added cleanup for byte string transformers, but the stage marked itself finished as soon as finish returned. If the final ByteString was waiting for downstream demand and downstream cancelled, postStop observed the finished flag and skipped cleanup.
The current Deflate and Gzip finish path already closes its Deflater before returning, so this follow-up hardens the generic stage lifecycle rather than claiming a second normal-path native leak.
Modification
Remove the finished flag and invoke the cleanup callback unconditionally from postStop, making cleanup the single finalizer for normal completion, failure, cancellation, and materializer shutdown.
Add directional StreamUtils tests for normal completion and for cancellation while a final emission is pending.
Result
Transformer resources are finalized on every stage termination path. The only production cleanup callback is idempotent, so normal completion still calls Deflater.end exactly once.
References
Refs #1133
Refs #1142