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 memory leak in Channel #2412

Merged
merged 1 commit into from
May 20, 2021
Merged

Fix memory leak in Channel #2412

merged 1 commit into from
May 20, 2021

Conversation

mpilquist
Copy link
Member

@mpilquist mpilquist commented May 20, 2021

Fixes both "hung merge" and "broadcastThrough identity". The unlessA breaks monadic tail recursion.

For posterity, here's how I found this one:

  • bisect pointed to ef57231 as the first commit for which "hung merge" test failed
  • that commit changed Stream.merge but none of the changes there changed the "shape" of streams besides swapping in Channel for Queue
  • by poking around in the .hprof written out by MemoryLeakSpec, I noticed the BindBind chains contained lots of references to Pull.eval(...).flatMap, so I searched Channel.scala for Pull.eval -- there were only 2 matches, one of which had the unlessA

Copy link
Contributor

@pchiusano pchiusano left a comment

Choose a reason for hiding this comment

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

Nice. This sounds like it might have been tricky to track down. :)

@SystemFw
Copy link
Collaborator

Oh, nice Mike! (and apologies)

@prova
Copy link

prova commented May 20, 2021

Yes, amazing, the tests pass now. We'll check the bigger codebase soon.

@mpilquist mpilquist deleted the bugfix/2408 branch February 18, 2024 13:34
@nikiforo nikiforo mentioned this pull request Jun 18, 2024
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.

5 participants