Skip to content

fix memory leak - #447

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

mapleeit merged 1 commit into
form-data:masterfrom
bodasia:patch-1

Conversation

@bodasia

@bodasia bodasia commented Oct 29, 2019

Copy link
Copy Markdown

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

bodasia commented Oct 29, 2019

Copy link
Copy Markdown
Author

@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

coveralls commented Oct 29, 2019 •

Copy link
Copy Markdown

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

bodasia commented Oct 30, 2019

Copy link
Copy Markdown
Author

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

bodasia commented Oct 30, 2019

Copy link
Copy Markdown
Author

@mapleeit Ok thanks.

@mapleeit mapleeit left a comment

Copy link
Copy Markdown
Collaborator

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