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

Question: Error: EMFILE: too many open files #26

Closed
Newan opened this issue Aug 12, 2017 · 8 comments
Closed

Question: Error: EMFILE: too many open files #26

Newan opened this issue Aug 12, 2017 · 8 comments
Assignees
Labels
question Question, clarification, discussion

Comments

@Newan
Copy link

Newan commented Aug 12, 2017

At first, thanks for this reader.

I want to read over 80k mp3 tags for my Software.
But after 7k the performance break down an finished with:

Error: EMFILE: too many open files....

Can someone help me to queue the async calls, with async or something like that:
https://caolan.github.io/async/docs.html#queue

Thanks

me code now is:

walker = walk.walk("/Volumes/..");
var mm = require('music-metadata');
const util = require('util');
walker.on("file", function (root, fileStats, next) {
 //todo, noch die anderen file extension freigeben
 if (path.extname(fileStats.name) === ".mp3") {
   mm.parseFile(path.join(root, fileStats.name)).then(function (metadata) {
     console.log(util.inspect(metadata, { showHidden: false, depth: null }));
   }).catch(function (err) {
     console.error(err.message);
   });
 }
 next();
});
@Borewit
Copy link
Owner

Borewit commented Aug 13, 2017

next() should be called after metadata has been parsed.

walker = walk.walk("/Volumes/..");
var mm = require('music-metadata');
const util = require('util');
walker.on("file", function (root, fileStats, next) {
  //todo, noch die anderen file extension freigeben
  if (path.extname(fileStats.name) === ".mp3") {
    next(); // Queue (asynchronous call) parsing of metadata
    mm.parseFile(path.join(root, fileStats.name)).then(function (metadata) {
      console.log(util.inspect(metadata, { showHidden: false, depth: null }));
      next(); // asynchronous call completed, 'release' next 
    }).catch(function (err) {
      console.error(err.message);
    });
  } else { // this else is required
    next(); // Go to next file, no asynchronous call 'queued'
  }
});

So this is I guess how to fix your existing code.
I use a recursion pattern most of the time (not necessary better) to solve that problem:

var fileStats // Array of stats of directory listing. 

// Take MP3 files from file listing
var mp3Stats = fileStats.filter(function () {
  return path.extname(fileStats.name) === ".mp3";
});

function parseFiles (root, mp3Stats) {
  var mp3Stat = mp3Stats.shift();
  if (mp3Stat) {
    mm.parseFile(path.join(root, mp3Stat.name)).then(function (metadata) {
      console.log(util.inspect(metadata, { showHidden: false, depth: null }));
      parseFiles(root, mp3Stats); // recursion
    }).catch(function (err) {
      console.error(err.message);
    });
  }
}

@Newan
Copy link
Author

Newan commented Aug 13, 2017

Thanks for your feedback, great.

For your information:
I have Mp3-Files there are not finished in the function und not in the error case. This mp3 file return nothing. I testet is in a single file and no output was generated. tThis mp3 files are corrupted and cant play in any mediaplayer. Is there an option to skip this, like an open timeout or something?

My working try:

if (path.extname(fileStats.name) === ".mp3") {
  console.log("Mp3 found: " + path.join(root, fileStats.name));

  var timer;
  mm.parseFile(path.join(root, fileStats.name)).then(function (metadata) {
    //console.log(util.inspect(metadata, { showHidden: false, depth: null }));
    console.log("Metadaten found: next()");
    next();
    clearTimeout(timer);
  }).catch(function (err) {
    console.error(err);
    console.log("Error on reading metadata: next()");
    next();
    clearTimeout(timer);
  });

  timer = setTimeout(function () {
    console.log('Timeout on file: ' + path.join(root, fileStats.name));
    next();
    timer = null;
  }, 3000);
} else {
  console.log("No mp3() found");
  next();
}

if on Mp3 has an error it is always:

TypeError: Cannot read property 'trim' of undefined
    at Function.Common.parseGenre (/Volumes/CardReader/Projects/project-desktop/dist/node_modules/music-metadata/lib/common.js:115:28)
    at MusicMetadataParser.setCommonTags (/Volumes/CardReader/Projects/project-desktop/dist/node_modules/music-metadata/lib/index.js:210:46)
    at MusicMetadataParser.parseNativeTags (/Volumes/CardReader/Projects/project-desktop/dist/node_modules/music-metadata/lib/index.js:126:22)
    at /Volumes/CardReader/Projects/project-desktop/dist/node_modules/music-metadata/lib/index.js:89:26
    at <anonymous>

Edit:

After Error 7254 files the walker loop ends and the next file to open get: EMFILE: too many open files. But there are over 80k mp3 over. without reading metatags loop works.

@Borewit
Copy link
Owner

Borewit commented Aug 13, 2017

Regarding first problem: After Error 7254 files the walker loop ends:

I assume the too many open files are caused by massive parallel parsing of metadata. In principal the code above looks correct to me.

Maybe ensure sequence by:

console.log('Start parsing file: %s', fileStats.name);
mm.parseFile(path.join(root, fileStats.name)).then(function (metadata) {
      //console.log(util.inspect(metadata, { showHidden: false, depth: null }));
      console.log("Metadaten found: next()");
      console.log('Finish parsing file: %s', fileStats.name);
      next();
      clearTimeout(timer);
      })

Ensure output is like:

Start parsing file: A
Finish parsing file: A
Start parsing file: B
Finish parsing file: B

Point is, that A should finish entirely, before B is started.

Regarding second problem: issue parsing for ever in corrupt MP3:

mm.parseFile returns a promise.

A promise should always resolve (callback in then) or reject (callback in catch).
Providing it with a corrupted MP3 file (it is amazing what a crappy files are going around), is not an excuse for being idle forever.

This might be a bug in the music-metadata module, if you attach this MP3 file (preferably as a new issue), I will have a look what is going on.

Putting timers around this is not the way to fix it.

@Borewit Borewit added the question Question, clarification, discussion label Aug 13, 2017
@Borewit
Copy link
Owner

Borewit commented Aug 13, 2017

Following code parsed 48k files without any issue:

const walk = require('walk')

var mm = require('music-metadata');
const util = require('util');
const path = require('path');

const walker = walk.walk("M:\\");

var filesParsed = 0;

walker.on("file", function (root, fileStats, next) {
  switch(path.extname(fileStats.name)) {
    case ".mp3":
    case ".flac":
      // Queue (asynchronous call) parsing of metadata
      mm.parseFile(path.join(root, fileStats.name)).then(function (metadata) {
        // console.log(util.inspect(metadata, { showHidden: false, depth: null }));
        console.log('Parsed %s files, last file: %s',  ++filesParsed, fileStats.name);
        next(); // asynchronous call completed, 'release' next
      }).catch(function (err) {
        console.error(err.message);
        next();
      });
      break;
    
    default:
      next(); // Go to next file, no asynchronous call 'queued'
  }
});

@Borewit Borewit self-assigned this Aug 13, 2017
@Newan
Copy link
Author

Newan commented Aug 14, 2017

thanks for your code. I test it on an Macbook pro 8 gb Ram. Tomorrow i will test it on Windows10

Parsed 8111 files, last file: 16-Mandela Day.mp3
Parsed 8112 files, last file: Cure - Pictures Of You.mp3

<--- Last few GCs --->

  113464 ms: Mark-sweep 1197.7 (1436.0) -> 1197.5 (1436.0) MB, 1687.6 / 0.0 ms [allocation failure] [GC in old space requested].
  115158 ms: Mark-sweep 1197.5 (1436.0) -> 1197.5 (1436.0) MB, 1694.1 / 0.0 ms [allocation failure] [GC in old space requested].
  117005 ms: Mark-sweep 1197.5 (1436.0) -> 1205.7 (1413.0) MB, 1846.0 / 0.0 ms [last resort gc].
  118843 ms: Mark-sweep 1205.7 (1413.0) -> 1213.8 (1413.0) MB, 1838.1 / 0.0 ms [last resort gc].


<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0x31828cbcfb51 <JS Object>
    2: NewPromiseCapability(aka NewPromiseCapability) [native promise.js:175] [pc=0x1d7459bbc5c3] (this=0x31828cb04381 <undefined>,O=0x31828cbc3059 <JS Function Promise (SharedFunctionInfo 0x31828cb74f51)>)
    3: then [native promise.js:~215] [pc=0x1d7459bcd434] (this=0x2b0bdec68bb9 <a Promise with map 0xd7dfc885551>,z=0x2b0bdec68db9 <JS Function (SharedFunctionInfo 0x3c340ca8f2a1)>,A=0x31828c...

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

can you post an email or something to send you an mp3 that failed to load

@Borewit
Copy link
Owner

Borewit commented Aug 14, 2017

You should have a message with my email address in your inbox.

@Borewit
Copy link
Owner

Borewit commented Aug 19, 2017

@Newan: Can you try v0.7.15?

@Newan
Copy link
Author

Newan commented Aug 21, 2017

Version 0.8.0 works even better. if the mp3 collection is free from scrap all works fine. If not, you have an problem not the lib...

Thanks for the great support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Question, clarification, discussion
Projects
None yet
Development

No branches or pull requests

2 participants