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

"drain" event listener leak when unpiping to response #135

Open
zbjornson opened this issue Mar 21, 2018 · 2 comments · May be fixed by #153
Open

"drain" event listener leak when unpiping to response #135

zbjornson opened this issue Mar 21, 2018 · 2 comments · May be fixed by #153
Assignees
Labels

Comments

@zbjornson
Copy link
Contributor

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.

@dougwilson dougwilson added the bug label Mar 28, 2018
@dougwilson dougwilson self-assigned this Mar 28, 2018
@matthiasg
Copy link

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', ...)

@zbjornson zbjornson changed the title "drain" event listener leak "drain" event listener leak when unpiping to response Mar 7, 2019
@zbjornson zbjornson changed the title "drain" event listener leak when unpiping to response "drain" event listener leak when unpiping to response, using once("drain") or removeListener("drain") Mar 7, 2019
@zbjornson zbjornson changed the title "drain" event listener leak when unpiping to response, using once("drain") or removeListener("drain") "drain" event listener leak when unpiping to response Mar 7, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 7, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 7, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 7, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 7, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 7, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 7, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 8, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 8, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 8, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 12, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 12, 2019
zbjornson added a commit to zbjornson/compression that referenced this issue Mar 12, 2019
dougwilson pushed a commit to zbjornson/compression that referenced this issue Jul 16, 2020
dougwilson pushed a commit to zbjornson/compression that referenced this issue Jul 16, 2020
dougwilson pushed a commit to zbjornson/compression that referenced this issue Jul 16, 2020
@jesseskinner
Copy link

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.

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

Successfully merging a pull request may close this issue.

4 participants