search

43081j / id3

A JavaScript ID3 tags parser for Node & browsers.
MIT License
335 stars 63 forks source link

need to close Reader after parsing is finished #13

Closed ntuckerxx closed 5 years ago

ntuckerxx commented 10 years ago

id3() creates a new Reader and calls its .open method but never closes it. If you don't cycle through lots of files quickly, this does not seem to be a problem (presumably because the Reader eventually gets destroyed and the file handle along with it), but if you churn through a lot of files very quickly, this can cause you to run out of file handles and id3 eventually starts failing with "Could not open specified file." The fix is simply to put a call to handle.close() inside the ID3Tag.parse callback.

43081j commented 10 years ago

There is a handle.close() in the ID3Tag.parse callback, here.

Did you mean elsewhere?

ntuckerxx commented 10 years ago

Ah, yes, but that version of the code is not what you get when you npm install id3js. So the bug still exists in the npm-published version. I guess this bug should be "publish 1.1.3 to npm please." :)

BTW, is there some reason you've excluded your 'build' directory from the repo? I've forked to fix another issue and I've been just editing the big concatenated 'dist/id3.js' file directly, but I assume that's not how you do it. I meant to contact you about this before sending a pull request, but I hadn't gotten around to it.

43081j commented 10 years ago

Sorry about that, its a bit of a mess.

Being that it began as a much smaller lib, I initially didn't have a solid build process. Then parts of it got forked off to other repos (dataview, reader.js) and now its a bit of a mess and a pain to update. Really, they should be deps now but we can't just npm them as this works for browsers too.

You can edit dist/id3.js but just remember some of the code is from dependencies, so if you feel the need to change those, there's likely a bug to be raised in their repos rather than this.