compression icon indicating copy to clipboard operation
compression copied to clipboard

"drain" event listener leak when unpiping to response

Open zbjornson opened this issue 6 years ago • 2 comments

This is sort of a weird scenario, but this test case fails:

  it('should clean up event listeners when res is unpiped to', function (done) {
    var listenerCount
    var server = createServer({ threshold: 0 }, function (req, res) {
      var times = 0
      var int = setInterval(function () {
        var rs = fs.createReadStream('does not exist')
        rs.on('error', function (e) {
          listenerCount = res.listenerCount('drain')
          rs.unpipe(res)
        })
        rs.pipe(res)
        if (times++ > 12) {
          clearInterval(int)
          res.end('hello, world')
        }
      })
    })

    request(server)
      .get('/')
      .set('Accept-Encoding', 'gzip')
      .expect(function () {
        assert.ok(listenerCount < 2)
      })
      .expect(200, done)
  })

I hit this in some code that retries creating a read stream until the source exists. We clean up from our side: rs.on("error", e => { rs.unpipe("res"); }). Seems like compression needs to be cleaning up its listeners when "unpipe" happens.

zbjornson avatar Mar 21 '18 21:03 zbjornson

there is also a leak caused by mapping ServerResponse.once to ServerResponse.on so there is no easy way to wait for 'drain' events to happen by just using the response.once('drain', ...)

matthiasg avatar Apr 17 '18 08:04 matthiasg

If anyone came here looking for a workaround for the ability to call res.once('drain', ...) without have node warn about MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 drain listeners added to [Gzip]. Use emitter.setMaxListeners() to increase limit..

This is a bit of a mess, but you can do something like this:

let onDrain;

// add a single drain listener early on
res.on('drain', () => {
    if (onDrain) {
        // call the onDrain callback at most once
        onDrain();
        onDrain = undefined;
    }
});

// later, when you would normally call res.once('drain', ...)
onDrain = () => {
    // resume your input stream or whatever
};

I hope that helps someone feeling stuck on this old issue.

jesseskinner avatar Mar 07 '22 18:03 jesseskinner