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.

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

Memory leak on large files with streaming middleware union #138

Open
buptsb opened this issue Mar 17, 2015 · 12 comments
Open

Memory leak on large files with streaming middleware union #138

buptsb opened this issue Mar 17, 2015 · 12 comments
Labels
needs-investigation stale

Comments

@buptsb
Copy link

buptsb commented Mar 17, 2015

I'm trying to serve large files(1 GiB) using http-server, the memory usage is abnormally high, so I got the heap profile and saw lots of 65535 KiB Buffer without being GCed.

smalloc
smalloc2

But when I switch to connect instead of union, everything is all right then, so I'm pretty sure that something wrong is happening in union.

http-server v0.7.5
node v0.10.32
@indexzero indexzero mentioned this issue Mar 19, 2015
Closed
@stianeikeland
Copy link

I'm seeing the same thing here. Requesting/streaming a large file (ex: a movie) will make it consume Gbs of memory. If I request the same file again the memory usage will double. Doesn't seem to be released, in the end the node process is killed by the OS when it runs out of available memory.

@roccomuso
Copy link

+1 I'm having the same issue when trying to transfer big files from devices with low-memory (like the Raspberry Pi).

Couldn't be related to this the issue? http://stackoverflow.com/questions/15001822/sending-large-image-data-over-http-in-node-js

@buptsb buptsb mentioned this issue Jan 20, 2016
Closed
@buptsb
Copy link
Author

buptsb commented Jan 20, 2016

Pull request flatiron/union#55, but seems this project is not maintained any more, a new dependency should be chosen.

@indexzero
Copy link
Collaborator

Yes. union is not maintained anymore on purpose since it predates streams2 in Node core. A PR that removes it would be welcome.

@derhuerst derhuerst mentioned this issue Mar 7, 2016
Closed
@derhuerst
Copy link

@indexzero

union is not maintained anymore […]. A PR that removes it would be welcome.

I'd be happy to do this. One solution might be to just use Express for the middleware-like behavior. A more lightweight solution might be to used middl. What do you think?

@hdf
Copy link

hdf commented Oct 8, 2016 •
edited
Loading

Problem persists on http-server 0.9.0 and node 6.7.0. Which is too bad, as otherwise this is a great little tool. Have to use HFS instead (http://www.rejetto.com/hfs/?f=dl). :( (For some reason HFS is faster as well, by about 20%.)

@jcrugzz
Copy link

jcrugzz commented Oct 22, 2016

This should be fixed with in [email protected]

@thornjad
Copy link
Member

Is this confirmed fixed? Can this issue be closed?

@tremby
Copy link

tremby commented Oct 26, 2018

I just tried transferring a huge file after running http-server 0.11.1 with no special command-line arguments and the memory usage stayed reasonable throughout. I don't know whether this answers the question, since there is no example code in this ticket and I don't know if union is being used by default.

@thornjad
Copy link
Member

union is still being used (though maybe it shouldn't be), but this issue looks to have been fixed with flatiron/union#54

@thornjad
Copy link
Member

We will eventually be removing union (see #483), so hopefully this issue will go away.

@thornjad thornjad mentioned this issue Apr 22, 2019
Open
cpetrov added a commit to eclipsesource/tabris-js-cli that referenced this issue Jan 29, 2020
@cpetrov
Migrate from union to connect
dd5692d 
Union is not actively developed anymore, see [1].

Union provides a middleware capability similar to connect [2] supporting
buffered streams and providing some convenience APIs on top of the
middleware capability. However, the stream functionality offered by
union is already available since Node.js v0.12 [3] and Tabris CLI does
not use most of the APIs union offered.

Migrate middleware handling to connect. Opt for connect instead of
express, since Tabris CLI does not use most of the extra features
express offers and express comes with significantly more external
dependencies (currently connect 4 vs express 30).

Error handling notes:
  * connect errors do not have a status field. Only log the error
    message.
  * connect does not handle 404 as an error as union did. Restore this
    behavior by using an extra middleware to catch all unhandled
    requests and treat them as errors.

[1]: http-party/http-server#138 (comment)
[2]: https://www.npmjs.com/package/connect
[3]: flatiron/union#61 (comment)

Fix #71

Change-Id: If502571e61f53098afd6cbfbfa66e8053691eeda
@github-actions
Copy link

This issue has been inactive for 180 days

@github-actions github-actions bot added the stale label Oct 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs-investigation stale
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@indexzero @tremby @jcrugzz @hdf @buptsb @stianeikeland @derhuerst @roccomuso @thornjad