Skip to content

fix memory leak#447

Merged
mapleeit merged 1 commit intoform-data:masterfrom
bodasia:patch-1
Oct 30, 2019
Merged

fix memory leak#447
mapleeit merged 1 commit intoform-data:masterfrom
bodasia:patch-1

Conversation

@bodasia
Copy link
Copy Markdown

@bodasia bodasia commented Oct 29, 2019

For many reasons, node caches and leaves in memory (for pretty lengthy amount of time) HTTPRequest instances. Form-data doesn't unsubscribe from 'error' and 'response' event, leaving references to finish / callback. This is kind-of okay in most cases (HTTPRequest's do die after a while), it might be a problem with large payloads or other memory-hungry situations.
This was only tested with NodeJS 8 - future versions might change how they are working with HTTPRequest's objects.

@bodasia
Copy link
Copy Markdown
Author

bodasia commented Oct 29, 2019

@mapleeit Hi can you help me to approve and merge this thing to project. I'll don't want to create post install script to fix this issue in my project. Thanks.

@bodasia bodasia force-pushed the patch-1 branch 2 times, most recently from f62a244 to b6acefb Compare October 29, 2019 15:38
@coveralls
Copy link
Copy Markdown

coveralls commented Oct 29, 2019

Coverage Status

Coverage increased (+0.05%) to 98.174% when pulling 2aaa0da on Bodasia:patch-1 into cd27e9a on form-data:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.04%) to 98.165% when pulling b72a6b1 on Bodasia:patch-1 into cd27e9a on form-data:master.

@bodasia
Copy link
Copy Markdown
Author

bodasia commented Oct 30, 2019

All issue of Travis in environment, not in source

@mapleeit
Copy link
Copy Markdown
Collaborator

@bodasia Thanks for the PR. I'll take a look later today!

For many reasons, node caches and leaves in memory (for pretty lengthy amount of time) HTTPRequest instances. Form-data doesn't unsubscribe from 'error' and 'response' event, leaving references to finish / callback. This is kind-of okay in most cases (HTTPRequest's do die after a while), it might be a problem with large payloads or other memory-hungry situations.
This was only tested with NodeJS 8 - future versions might change how they are working with HTTPRequest's objects.
@bodasia
Copy link
Copy Markdown
Author

bodasia commented Oct 30, 2019

@mapleeit Ok thanks.

Copy link
Copy Markdown
Collaborator

@mapleeit mapleeit left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants