Memory usage and leak problem with large file uploads

[screenmeet]

The current implementation blows up the process memory on large file uploads (try with 1GB+ file, for example, and monitor process memory).

We made a PR which uses temporary files to manage the intermediate file uploads instead of using memory.

#88

Enjoy!

[kyleratti]

Wow, god bless you! I've been going nuts trying to track down a memory leak when uploading files to my API. It works fine locally on my Macbook, but in production on my Ubuntu servers it just eats the RAM until Node crashes. The files are only around 70MB in size, too, so much lower than what you're seeing, although Node is running on a VM with 1GB of RAM.

You're the man!

[screenmeet]

@kyleratti You're welcome.

I also observed some different behaviors on different node versions / OS' in terms of memory leakage with the buffers. Sometimes it would just take 2x the file size (70mb would take 140mb), other times, it would jump almost exponentially (a 100MB file would eat 700MB ram).

There was also a secondary issue where if a file was aborted, those intermediate chunks will just live in memory forever (until the process crashes). That issue will still persist if you are using the memory-based approach. I strongly recommend using the intermediate files and not memory buffers...

[kyleratti]

@screenmeet yeah, that's what was so confusing about debugging this. The absolute only difference between my environments, aside from OS, was that the final storage was on a NFS-mounted share - but express wasn't even calling my route handler, so there was no way that was related.

I wonder if it makes more sense to save to /dev/shm and then move it?

[screenmeet]

@kyleratti I wouldn't try putting intermediate files anywhere in memory... just too easy to eat eat up all available memory if there are many concurrent uploads or people are uploading large files. Major DDoS vulnerability if you ask me.

[kyleratti]

@screenmeet that's true - my use-case is a private API used to connect a few of my services together so I don't have that concern, but I see your point. You get the speed advantage with /dev/shm going to memory but you're right, you do have to have a chunk of memory reserved for that alone.

[richardgirges]

Hi @screenmeet, still awaiting linting fixes in the PR. There are also some conflicts now. In the meantime, just wanted to notify everyone that the latest v1.0.0-alpha.1 release should address some memory issues. We are no longer building md5 checksums on every upload as this was non-performant and taxing on memory.

[richardgirges]

Resolve by #109