"drain" event listener leak when using res.once("drain"); can't use res.removeListener("drain")

[zbjornson]

This module intercepts res.on and remaps it to stream.on. Using res.once("drain", listener) multiple times creates a listener leak because this is essentially what happens:

res.once("drain", listener);
// expands to this (see https://github.com/nodejs/node/blob/aec34473a7eddae32e6c359715ce521446066210/lib/events.js#L282-L303)
res.on("drain", () => {
  res.removeListener("drain", listener);
  listener();
});
// but compression intercepts `on`, so you wind up with:
stream.on("drain", () => {
  res.removeListener("drain", listener); // <-- wrong target
  listener();
})

For that same reason, it looks like a listener bound with res.on("drain", listener) can never actually be removed with res.removeListener.

(Also mentioned in https://github.com/expressjs/compression/issues/135#issuecomment-381899124.)

[dougwilson]

duplicate of #135

please correct me if this is not a duplicate ticket

[zbjornson]

I guess it's the same underlying cause, but two new manifestations.

[dougwilson]

Gotcha. I assume that the Node.js .unpipe logic is just calling removeListener under the hood, in which case the fix being to fixup removeListener would fix these cases, I would guess. It seems this issue is mentioned in #135 which would help drive whoever picks up the bug to fix to get it covered in a generic way. You could add your above snippet to that issue as well, if you think that would be useful to someone who picks up #135