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

Make ManualPromise able to hand out a leak-safe "settler." #171

Open
danfuzz opened this issue Mar 23, 2023 · 0 comments
Open

Make ManualPromise able to hand out a leak-safe "settler." #171

danfuzz opened this issue Mar 23, 2023 · 0 comments
Labels
bug Something isn't working framework Has something to do with the system framework

Comments

@danfuzz
Copy link
Owner

danfuzz commented Mar 23, 2023

Describe the bug

It's at least theoretically possible for a ManualPromise to be complicit in a memory leak, in cases where an instance is only needed so that one of its settling methods can be called. Once an instance has actually been settled, then it can't possibly be settled again, and that means that a "settler" object (a) doesn't need to refer to the original ManualPromise nor to the underlying (regular) promise, nor to the settled value; and (b) if it does refer to any of those and is a long-lived object, then that "settler" would be a source of a leak.

In the current implementation, there is no separate "settler" object from a ManualPromise instance, and ManualPromise itself necessarily maintains a reference to the settled value. So, the possibility of a leak is kinda inherent in the design. This can be fixed.

To reproduce

Not yet known.

Expected behavior

No leaks can occur due to reasonable uses of this class.

Actual behavior

A leak occurs.

Environment

  • OS: Any
  • Node: Any

Additional context

This issue was inspired by nodejs/node#17469 (comment), see which for details.

@danfuzz danfuzz added bug Something isn't working framework Has something to do with the system framework labels Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working framework Has something to do with the system framework
Projects
None yet
Development

No branches or pull requests

1 participant