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

Fixed memory leak in Promise#cancellableGet #1027

Merged
merged 4 commits into from
Dec 19, 2017

Conversation

mpilquist
Copy link
Member

@mpilquist mpilquist commented Dec 19, 2017

The old implementation of cancellableGet had two flaws:

  • each invocation of force would register a callback under an internal id, overwriting any previously registered callback for the same id
  • cancel would simply remove the registration for the id

For example, cancel *> force would result in a state where a callback for the force was registered. force *> force *> force would result in a similar state, where the first two force actions would never be completed.

To fix, the internal state has been changed so that we track a list of callbacks associated with each id. Calling cancellableGet registers an empty list under that id. Each force adds a new callback to the list, but only if that id is still in the map. Calling cancel removes the id from the map.

@mpilquist
Copy link
Member Author

Fixes #1026

Copy link
Collaborator

@SystemFw SystemFw left a comment

Choose a reason for hiding this comment

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

Nice idea

@mpilquist mpilquist merged commit 5d715e0 into typelevel:series/0.10 Dec 19, 2017
@mpilquist mpilquist deleted the topic/promise-leak branch February 18, 2020 12:57
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.

2 participants