8000 End handlers not being removed after next resolve · Issue #5 · basicdays/node-stream-to-async-iterator · GitHub
[go: up one dir, main page]
Skip to content

End handlers not being removed after next resolve #5

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.

Sign up for GitHub

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

Jump to bottom
Closed
AKST opened this issue Jul 16, 2017 · 2 comments · Fixed by #6 or #11
Closed

End handlers not being removed after next resolve #5

AKST opened this issue Jul 16, 2017 · 2 comments · Fixed by #6 or #11

Comments

@AKST
Copy link
Contributor
AKST commented Jul 16, 2017

In the notReadable state it's listening for either the end or read callback to finish, unfortunately if the read callback fires it doesn't clean up the end callback. And after 11 calls to next there are 10 event listeners for the end callback.

I know in my node version I get warned about possible memory leaks.

Suggestion

The existing logic here

await Promise.race([this._untilReadable(), this._untilEnd()]);

Could possibly changed to this

const { promise: readPromise, cleanup: cleanRead } = this._untilReadable();
const { promise: endPromise,  cleanup: cleanEnd } = this._untilEnd();
await Promise.race([readPromise, endPromise]);
cleanRead();
cleanEnd();

The cleanUp callbacks will unhook any dangling event listeners
@AKST AKST mentioned this issue Jul 16, 2017
Merged
basicdays added a commit that referenced this issue May 30, 2018
@basicdays
Update babel version, use babel transform runtime
965ea8b 
Switching to using babel transform runtime solves issue #5.
@basicdays
Copy link
Owner

I haven't released this fully yet, but I did publish a beta version of this that also fixes a couple other issues. If you would like to test it out, it's available under stream-to-async-iterator@beta.

@basicdays basicdays mentioned this issue Mar 16, 2022
Merged
basicdays added a commit that referenced this issue Mar 16, 2022
@basicdays
chore: Overhaul and update project (#11)
3b1f109 
-   Breaking Change: Dropping support for Node 11 and below.
-   Breaking Change: Dropping support for Flow.
-   Bug Fix: Resolves #5. Event handlers are cleaned up after each iteration, fixing some memory leak issues.
-   Bug Fix: Resolves #7. Code no longer depends on babel runtimes or regenerator.
-   Bug Fix: Handles stream not buffering due to starving the event loop.
-   Feature: Resolves #1. Now handles `.throw` and `.return` hooks on the async iterator.
-   Feature: Properly closes stream when finished.
-   Feature: Added support for TypeScript.
-   Feature: Support for Node.js versions 12, 14, and 16.
-   Chore: General overhaul of project setup (should not impact what is published).
@basicdays
Copy link
Owner

Released in v1.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Removes end event listens
Overhaul and update project basicdays/node-stream-to-async-iterator
2 participants
@basicdays @AKST
0