Skip to content

Fix regression in signal unlisten and document current EE behaviour

process.removeAllListeners() doesn't work for signals. It should, and it used to, prior to https://github.com/iojs/io.js/commit/56668f54d11225b0bcdf93dc2fba846febdbb19f

I originally PRed a fix for this in https://github.com/joyent/node/pull/8953. That fix is a one-liner, wherein a subtle optimization in EventEmitter.removeAllListeners() is evaded by a seeming no-op process.on('removeListener', function(){}), which is pretty awful from a comprehensibility point of view.

src/node.js is simpler and more understandable if it just uses the new/remove listener events, but they are theoretically useless for this kind of use-case, since the documentation is officially weasely about whether they fire before or after the listener is actually added. see: https://github.com/joyent/node/issues/8853 and https://github.com/piscisaureus/io.js/commit/ff350f716971e0a7d93e7631d38891682a2da75b

I've never seen the new/remove listener used for any purpose other than what node itself could use it for: detecting whether there is indeed a listener for an event, and starting or stopping some processing accordingly. It should be easier to use the EE for this.

Even if you don't agree the indeterminancy in the EE API should be removed, node is free to depend on implementation details of itself, so 90a5e2b doesn't require c1d5fab. For node to assume that the EE behaves as it does seems better than my original fix/hack in https://github.com/sam-github/node/commit/06b6e75668ae9b4f57a61d5b48633ae7d3f8f434

R=@bnoordhuis

Merge request reports

Loading