Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: stream leak in akka-http client backend for indefinite server st… #1833

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

steffenhaak
Copy link
Contributor

Fixes #1832

Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, would be good with test coverage.

Could be added in interop-tests and have a client and server where the server closes over a promise, uses .watchTermination on the response stream to complete the promise and future.failed.futureValue as assert in the test case to wait for client cancelation to be seen on the server side.

@johanandren
Copy link
Member

I've pushed a test that I have verified locally fails on main and passes with your fix.

@johanandren johanandren added this to the 2.4.0-M1 milestone Sep 15, 2023
@johanandren johanandren force-pushed the fix-akka-http-backend-stream-leak branch from e8b4937 to 86e481f Compare September 15, 2023 15:26
@steffenhaak
Copy link
Contributor Author

Great, thank you. Next time I submit a PR, I will try to provide an appropriate test as well.

@patriknw
Copy link
Member

fixed a Scala 3 compilation error in the test

Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johanandren johanandren merged commit 77b8e49 into akka:main Sep 20, 2023
12 checks passed
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.

Stream leak in akka-http backend (grpc client)
3 participants