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

Memory leak by parsing special mp3 file #34

Closed
Newan opened this issue Aug 21, 2017 · 7 comments
Closed

Memory leak by parsing special mp3 file #34

Newan opened this issue Aug 21, 2017 · 7 comments
Assignees
Labels
bug Bug, will addressed with high priority

Comments

@Newan
Copy link

Newan commented Aug 21, 2017

On parsing special single file, the node enviremont used more then 1,4 gb of ram and exit with:

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory

@Borewit I sended you one file by mail for test, if u or another want more example please contact me.

@Borewit
Copy link
Owner

Borewit commented Aug 24, 2017

Will give an update on this annoying problem.

This typically happens on corrupted MP3 files without any ID3v2 header / Xing header.

It will then scan the MPEG frame., and jump from one frame to the next frame.
But if these frames contain garbage, it needs to re-sync, to look for the next valid frame.

Re-syncing is done by searching byte by byte.

The asynchronous nature of JavaScript, in combination with promise wrapped recursion loop, will result on a zero-wised (corrupt MP3 file ;-) file of 3 MB, in a recursion depth of ~3 million.
This in combination with the actual file read, is apparently extremely expensive for Node.

I don't see a a nice solution, I am pretty sure it not caused by some kind of mistake.

Two solutions I can think of are:

  1. Give up after x amount of unsynced bytes
    • will break other stuff, cause will not complete find all information (metadata, format info)
  2. Cache bigger chunks of data:
    • ~10 faster, probably keeps memory under control
    • Ugly code, looks so overcoplicated for something stupid to read until a byte sequence has been found

Related/cause: strtok3 issue #5

@Newan
Copy link
Author

Newan commented Aug 25, 2017

i analysed this issues in the last few days and found an handfull mp3 in my collection with this error. But there are files they are not corrupt for other programms like vlc. windows explorer can read the id-tags also. if you want i can send you some oof this files too.

Now it is for me not important which solution you pick, important for me is that one of the solution stop this memory overrun.

thanks for your support!

@Newan
Copy link
Author

Newan commented Aug 31, 2017

o.k it seems that is not so easy to fix. my workaround is to make an optional parameter like execution time. if in mp3 is in this time read everythink is o.k if an mp3 need more time, it will be skippt or genrate an error like, excution time exceeded.

so we have no problem to read many mp3 files in a loop

@Borewit
Copy link
Owner

Borewit commented Sep 1, 2017

Good to hear you found a workaround. You could also use unsynced, counting the number of unsychronized bytes in MpegParser.ts

There is indeed not an easy fix. I don't want to go in that direction for a permanent solution.
In the future, I would like to make data corruption part of the result.

I solved the issue, solution only has some side effects, once that is fixed I will make it available.

Borewit added a commit that referenced this issue Sep 2, 2017
@Borewit
Copy link
Owner

Borewit commented Sep 2, 2017

Addressed in v0.8.2

@Newan & @mycoboco : would be great if you could give it a try.

@Newan
Copy link
Author

Newan commented Sep 2, 2017

For me the first try is very great !

Fast and sure! If i found some errors i will reopen this issues or make a new one!

Thanks, great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, will addressed with high priority
Projects
None yet
Development

No branches or pull requests

2 participants