browserify / factor-bundle

factor browser-pack bundles into common shared bundles
Other
402 stars 27 forks source link

warning: possible EventEmitter memory leak detected #64

Open benoitguigal opened 9 years ago

benoitguigal commented 9 years ago

I am using factor-bundle with watchify and gulp.

var gulp = require('gulp');
var gutil = require('gulp-util');
var source = require('vinyl-source-stream');
var browserify = require('browserify');
var reactify = require('reactify');
var watchify = require('watchify');
var factor = require('factor-bundle');
var uglify = require('gulp-uglify');
var fs = require('fs');
var concat = require('concat-stream');
var file = require('gulp-file');

gulp.task('watch', bundle)

function bundle () {

  // react components
  var files = [
    '/path/to/file1.jsx',
    '/path/to/file2.jsx',
    '/path/to/file3.jsx'
  ];

  var bundler = watchify(browserify(watchify.args)) 

  bundler.add(files);
  bundler.add('./lib/api.js', {expose: 'api'});
  bundler.require('./lib/api.js', {expose: 'api'});
  bundler.transform('reactify');
  bundler.on('update', rebundle);

  function rebundle() {
    bundler.plugin('factor-bundle', {
        outputs: [
          write('/path/to/file1.js'),
          write('/path/to/file2.js'),
          write('/path/to/file3.js'),
          ]
    });
    bundler.bundle()
        .on('error', gutil.log.bind(gutil, 'Browserify Error'))
        .pipe(write('shared.js'));
  };

  return rebundle();
}

function write (name) {
    return concat(function (content) {
        // create new vinyl file from content and use the basename of the
        // filepath in scope as its basename.
        return file(name, content, { src: true })
        // uglify content
        .pipe(uglify())
        // write content to build directory
        .pipe(gulp.dest('./public/bundles/'))
    });
}

It works fine but after five or ten rebundles I start seeing the following warning:

Trace
    at Browserify.addListener (events.js:179:15)
    at f (/Users/benoit/git/figure/web/node_modules/factor-bundle/index.js:55:7)
    at Browserify.plugin (/Users/benoit/git/figure/web/node_modules/browserify/index.js:345:9)
    at Browserify.bundle (/Users/benoit/git/figure/web/gulpfile.js:46:13)
    at Browserify.emit (events.js:107:17)
    at null._onTimeout (/Users/benoit/git/figure/web/node_modules/watchify/index.js:126:15)
    at Timer.listOnTimeout (timers.js:110:15)
(node) warning: possible EventEmitter memory leak detected. 11 finish listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at ConcatStream.addListener (events.js:179:15)
    at ConcatStream.once (events.js:204:8)
    at Labeled.Readable.pipe (/Users/benoit/git/figure/web/node_modules/factor-bundle/node_modules/labeled-stream-splicer/node_modules/stream-splicer/node_modules/readable-stream/lib/_stream_readable.js:612:8)
    at /Users/benoit/git/figure/web/node_modules/factor-bundle/index.js:73:43
    at Array.reduce (native)
    at Transform._flush (/Users/benoit/git/figure/web/node_modules/factor-bundle/index.js:65:35)
    at Transform.<anonymous> (/Users/benoit/git/figure/web/node_modules/factor-bundle/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:135:12)
    at Transform.g (events.js:199:16)
    at Transform.emit (events.js:129:20)
    at finishMaybe (/Users/benoit/git/figure/web/node_modules/factor-bundle/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:371:12)
    at endWritable (/Users/benoit/git/figure/web/node_modules/factor-bundle/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:378:3)
    at Transform.Writable.end (/Users/benoit/git/figure/web/node_modules/factor-bundle/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:356:5)
(node) warning: possible EventEmitter memory leak detected. 11 finish listeners added. Use emitter.setMaxListeners() to increase limit.
neilcarpenter commented 9 years ago

+1

Freyert commented 9 years ago

Yeah, this is for real. Every time the plugin is called the addHooks event listener is attached to Browserify. As you can see in this illustration

 [ [Function: collect],
  [Function: reset],
  [Function],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks],
  [Function: addHooks] ]

Just add: bundler.on('reset', function () { gutil.log(bundler.listeners('reset')); });, to see the results yourself. I'm guessing this probably leads to the excessive listeners on ConcatStream.

Freyert commented 9 years ago

@benoitguigal to fix this do this to your rebundle function:

  var outputs;
  bundler.plugin('factor-bundle', {
    outputs: outputs
  }); 
  function rebundle() {
    outputs = files.map(function (file) {
      var name = path.basename(file, '.jsx');
      return write("./" + name + '.js');
    }); 

    bundler.bundle()
    .on('error', gutil.log.bind(gutil, 'Browserify Error'))
    .pipe(write('shared.js'));
  }

And please give me that bounty on SO :smiley:

terinjokes commented 9 years ago

This seems like a real bug, looking into it. addHooks should only be added once, you can see it's the saem logic as in watchify: https://github.com/substack/watchify/blob/e20c9a39fad48f411b2dc48a1d784a947bb8fb9e/index.js#L59-L60

Freyert commented 9 years ago

So my SO solution doesn't work right. Luckily I have another solution in the interim until a PR is made.

So try this: Edit node_modules/factor-bundle/index.js and change

From b.on('reset', addHooks); to b.once('reset', addHooks);

Your original code should work.

benoitguigal commented 9 years ago

Thanks Freyert, I will give it a try :-)

neilcarpenter commented 9 years ago

Cheeeers, has fixed for me

Freyert commented 9 years ago

BTW @benoitguigal and @neilcarpenter nice sites/projects.

neilcarpenter commented 9 years ago

Haha thanks!! :) And yeah, ditto to @benoitguigal

johnnyshankman commented 9 years ago

+1, is there still no PR or merged code for this?

kennethaasan commented 9 years ago

+1

davidchase commented 8 years ago

this seems to be an issue still with 2.5.0 on Mac OSX

browserify ./client/src/app/*.js -p  [ factor-bundle  -o 'uglifyjs -cm > ./client/dist/js/`basename $FILE`' ] -o ./client/dist/js/common.js

the above "fix" does not solve the issue...

johnnyshankman commented 8 years ago

This was still a problem I had in my grunt karma task

Running "karma:ci" (karma) task
(node) warning: possible EventEmitter memory leak detected. 11 bundled listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at Browserify.addListener (events.js:179:15)
    at Browserify.once (events.js:204:8)
    at deferredBundle (/Users/user/Documents/feed/node_modules/karma-bro/lib/bro.js:183:11)
    at Browserify.w.bundleFile (/Users/user/Documents/feed/node_modules/karma-bro/lib/bro.js:243:7)
    at /Users/user/Documents/feed/node_modules/karma-bro/lib/bro.js:269:9
    at nextPreprocessor (/Users/user/Documents/feed/node_modules/karma/lib/preprocessor.js:48:28)
    at /Users/user/Documents/feed/node_modules/karma/lib/preprocessor.js:86:7
    at evalmachine.<anonymous>:334:14
    at /Users/user/Documents/feed/node_modules/karma/node_modules/graceful-fs/graceful-fs.js:102:5
    at FSReqWrap.oncomplete (evalmachine.<anonymous>:95:15)
Injecting Rewireify into modules

I got it to stop by trying this line for an arbitrary number n:

require('events').EventEmitter.prototype._maxListeners = n;

I continued to get warnings that I had n+1 bundled listeners until I finally set _maxlistners to 103 or larger.

So if you're willing to sit down and find out how many listeners you've been adding, you can at least silence the warning by using the line above somewhere in the global scope.

Emnalyeriar commented 8 years ago

I defined my browserify + watchify task exacly as here and got this problem as well.

(node) warning: possible EventEmitter memory leak detected. 11 reset listeners added. Use emitter.setMaxListeners() to increase limit.
Liero commented 8 years ago

Why this has not been fixed yet?

Liero commented 8 years ago

I used this ugly workaround, since I was not able to extract the plugin outside rebundle function

** 
 * fixes possible EventEmitter memory leak detected: https://github.com/substack/factor-bundle/issues/64
 * @param {Browserify.BrowserifyObject} bundler
 */
function fixFactorBundleMemoryLeak(bundler) {
  var listeners = bundler.listeners("reset").filter(l => l.name === "addHooks");
  for (var i = 0; i < listeners.length - 1; i++) { //remove "reset" listeners except last
    bundler.removeListener('reset', listeners[i]);
  }
}