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.

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

function detectSize is causing memory leak #84

Closed
Karmel0x opened this issue Sep 25, 2021 · 1 comment
Closed

function detectSize is causing memory leak #84

Karmel0x opened this issue Sep 25, 2021 · 1 comment

Comments

@Karmel0x
Copy link

Karmel0x commented Sep 25, 2021

this function is causing memory leak

function detectSize (cb) {
stream = pipeline(detectSize((size) => {

my case: uploading many huge files, just one after the other

passing size option to upload function resolves the problem, as it's not using this function then

@qgustavor
Copy link
Owner

qgustavor commented Sep 25, 2021

That's a functionality from tonistiigi's mega. When I converted tonistiigi's library into ES6 I remember to have noticed that the way this function works is not that great, as it puts everything in memory. I can't disable this functionality without breaking compatibility and the solution, as you said, is just specifying the size. Documentation for this function says: "Note that because MEGA's API needs final data length before uploading can start, streaming only fully works if you specify the size of your data. Otherwise it needs to first buffer your data to determine the size"

Even if this pitfall is documented, it's pretty easy to forget it: even megajs-cli have this issue (and I think I never noticed it because I hardly ever upload large files).

I think that is not ideal changing the library to remove options that would break backyards compatibility now, as there is a plan to rewrite the library, even if it will take a long time as I'm not planning to do it alone without community feedback, mostly because I don't know what streaming pattern to use: there are Node streams, W3C Streams API and Async Generators, each one with advantages and drawbacks.

I hope that in the rewrite those pitfalls get removed, like disallowing uploading without specifying a size first and disallowing specifying encryption keys (which is pretty bad as there was a famous Mega client using a constant all zeroes key on uploads instead of using safety random generated keys, making Mega security worse for those who use this client and for those who download files uploaded using this client).

qgustavor added a commit that referenced this issue Jan 20, 2022
But is needed for the greater well being of everyone
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

No branches or pull requests

2 participants