EventEmitter memory leak warning

[pushred]

I'm using gulp-run in conjunction with gulp-tap to pass the paths of files I'm watching on to a theme upload command in shopify_theme Ruby gem. Generally this works but I've started seeing the following warning as the number of files in my project increases. The uploads are still successful, but this is making for some noisy log output.

var gulp = require('gulp');
var sass = require('gulp-ruby-sass');
var run = require('gulp-run');
var tap = require('gulp-tap');

var paths = {
  styles: 'assets/styles/**/*'
};

var upload = function( file ){
  var splitPath = file.path.split('theme/').pop();
  run('theme upload ' + splitPath, { cwd: 'theme' }).exec();
};

gulp.task('sass', function(){
  gulp.src(paths.styles)
    .pipe(sass())
    .pipe(gulp.dest('theme/assets'));
    .pipe(tap(function(file){
      upload(file);
    }));
});

gulp.task('watch', function(){
  gulp.watch(paths.styles, ['sass']);
});

(example gulp-run log output)

$ theme upload assets/timber.scss.liquid
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at WriteStream.EventEmitter.addListener (events.js:160:15)
    at WriteStream.Readable.on (_stream_readable.js:688:33)
    at WriteStream.EventEmitter.once (events.js:185:8)
    at Duplex.Readable.pipe (_stream_readable.js:540:8)
    at exec (/project/node_modules/gulp-run/gulp-run.js:90:17)
    at Transform.commandStream.exec (/project/node_modules/gulp-run/gulp-run.js:204:12)
    at upload (/project/Gulpfile.js:18:54)
    at /project/Gulpfile.js:39:7
    at Stream.modifyFile (/project/node_modules/gulp-tap/lib/tap.js:66:11)
    at Stream.stream.write (/project/node_modules/gulp-tap/node_modules/event-stream/node_modules/through/index.js:26:11)

Not sure if this is related to https://github.com/gulpjs/gulp/issues/432 but seems to be coming from Transform.commandStream.exec in gulp-run. I'm running 0.10.28.

[cbarrick]

The warning is "normal" and simply means you're processing more than 10 files in parallel. gulp-run spawns a new process to handle every file it receives as input and processes them in parallel. For each file, a logger object is created to handle piping the stdout of the child process to the stdout of the main process. Piping involves creating event listeners on process.stdout, hence the warning.

This log system could definitely be refactored to avoid this, but no memory is actually being leaked (as of 93030182533a5abbc6a1621849c8a35cf0d34583), so you can safely increase the listener count on process.stdout to remove the warning.

Consider the following two examples that run test a = a one-hundred times.

Parallel Example

var count = 100;
function almostDone() {
    count -= 1;
    if (count === 0) done();
}

for (var i = count; i > 0; i -= 1) {
    run('test a = a').exec(almostDone);
}

Since exec is asynchronous, this example spawns 100 processes in parallel and calls done when they have all exited. You'll see the memory leak warning here because all 100 of the processes attach listeners to process.stdout. Since gulp pipelines are asynchronous, this is what happens and is usually what should happen from a performance standpoint.

Series Example

var count = 100;
function test() {
    run('test a = a').exec(function () {
        count -= 1;
        if (count > 0) test();
        else done();
    });
}
test();

This example also spawns 100 processes, but it waits for the previous to finish before spawning the next. In this case, you won't see the warning because each process releases it's listeners before the next process starts. And since the warning is not thrown, this example shows that each execution properly cleans up after itself.

The solution

As far as I can tell, the solution is process.stdout.setMaxListeners(n + 1) where n is the number of files being processed. Or just process.stdout.setMaxListeners(Infinity).

[pushred]

Awesome, thanks so much for this detailed explanation. This all makes perfect sense. The concurrency is definitely desirable!

[cbarrick]

I recently published a rewrite that avoids the issue altogether. See v1.6.0 and the updated readme.