zio / zio-http

A next-generation Scala framework for building scalable, correct, and efficient HTTP clients and servers
https://zio.dev/zio-http
Apache License 2.0
768 stars 390 forks source link

ZClient memory leaks finalizers on each request #2956

Closed ntngel1 closed 1 week ago

ntngel1 commented 1 month ago

Describe the bug We are using ZIO HTTP in production and have encountered an issue with a constantly increasing amount of memory consumed by the service. At some point, the service is unable to free up memory and subsequently restarts.

Screenshot 2024-07-12 at 13 45 44

We investigated this issue by taking a memory dump and inspecting it using Eclipse Memory Analyzer. Here is the result:

Screenshot 2024-07-12 at 13 51 04

We checked the zio.Scope class which has a Running state with finalizers: LongMap[Finalizer]. We set a breakpoint at zio.Scope:156 within the addFinalizerExit method and discovered that each ZClient request adds a new finalizer but does not remove it. The longer the application runs, the more finalizers are added.

Screenshot 2024-07-12 at 13 41 27

You can download this memory dump for investigation.

To Reproduce Steps to reproduce the behavior:

  1. Clone this demo project to reproduce the problem: https://github.com/ntngel1/zio-http-zclient-leak
  2. Set a breakpoint at zio.Scope:156 in the addFinalizerExit method.
  3. Run the program.
  4. Let it run for 10 seconds (it makes 10 requests). Attach a debugger and you will see that there are 10 finalizers added and none removed. The more requests made, the more finalizers are added, leading to increased memory consumption.

Expected behaviour Finalizers should be removed after they are no longer needed to prevent memory leaks.

kyri-petrou commented 1 month ago

Hey @ntngel1, thanks for reporting. I think this one is to be blamed on incomplete documentation and suboptimal UX design of the ZClient.

I believe the issue in the reproducer is that the Scope is not being closed after handling of the request, since you're providing a layer that attaches the finalizes to the global Scope.

Can you try this and let me know if the memory leak still exists?

  val program = ZIO.scoped(for {
    res <- ZClient.request(Request.get(url).addHeaders(headers))
    data <- res.body.asString
    _ <- Console.printLine(data)
  } yield ())

In short, you should be wrapping ZClient.request calls in ZIO.scoped, but only after you've extracted the body from the response.

987Nabil commented 1 month ago

@kyri-petrou what do you think, should we offer a API that we promote as the default, that works more like acquireReleaseWith? So you have to handle the body in a callback.

ntngel1 commented 1 month ago

Hey @ntngel1, thanks for reporting. I think this one is to be blamed on incomplete documentation and suboptimal UX design of the ZClient.

I believe the issue in the reproducer is that the Scope is not being closed after handling of the request, since you're providing a layer that attaches the finalizes to the global Scope.

Can you try this and let me know if the memory leak still exists?

  val program = ZIO.scoped(for {
    res <- ZClient.request(Request.get(url).addHeaders(headers))
    data <- res.body.asString
    _ <- Console.printLine(data)
  } yield ())

In short, you should be wrapping ZClient.request calls in ZIO.scoped, but only after you've extracted the body from the response.

Tried it out, and it works πŸ‘ However, it's not immediately obvious that we need to wrap each request using ZIO.scoped(). This wasn't mentioned in the documentation, and all examples were written using the global Scope. I think it would be better and less error-prone if the library's API took care of this for developers and defined everything explicitly. This way, everyone would use acquireReleaseWith, and there would be less room for error.

kyri-petrou commented 1 month ago

@kyri-petrou what do you think, should we offer a API that we promote as the default, that works more like acquireReleaseWith? So you have to handle the body in a callback.

I'm 100% onboard with this πŸ™

jdegoes commented 1 month ago

/bounty $175 for fix, doc updates, and tests.

algora-pbc[bot] commented 1 month ago

πŸ’Ž $175 bounty β€’ ZIO

Steps to solve:

  1. Start working: Comment /attempt #2956 with your implementation plan
  2. Submit work: Create a pull request including /claim #2956 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to zio/zio-http!

Add a bounty β€’ Share on socials

Attempt Started (GMT+0) Solution
🟒 @kyri-petrou Aug 7, 2024, 2:05:47 PM #3008
kyri-petrou commented 1 month ago

/attempt #2956

I'll be working on this alongside #2957

Algora profile Completed bounties Tech Active attempts Options
@kyri-petrou    23 ZIO bounties
+ 2 bounties from 1 project
Scala, Python,
Shell & more
﹟2957
Cancel attempt
algora-pbc[bot] commented 1 month ago

πŸ’‘ @kyri-petrou submitted a pull request that claims the bounty. You can visit your bounty board to reward.

algora-pbc[bot] commented 1 week ago

πŸŽ‰πŸŽˆ @kyri-petrou has been awarded $175! 🎈🎊