fix: handled case where event handler removed inside handler #126
Conversation
mqemitter.js
Outdated
| @@ -118,7 +118,7 @@ MQEmitter.prototype._do = function (message, callback) { | |||
| const matches = this._matcher.match(message.topic) | |||
|
|
|||
| this.current++ | |||
| this._parallel(this, matches, message, callback) | |||
| this._parallel(this, Array.from(matches), message, callback) | |||
There was a problem hiding this comment.
I have a feeling that Array.from is slower than allocating an array of the same size and then use an old school for (;;) to copy over.
mqemitter.js
Outdated
| const matchesCopy = new Array(matches.length) | ||
| for (let i = 0, len = matches.length; i < len; i++) { | ||
| matchesCopy[i] = matches[i] | ||
| } |
There was a problem hiding this comment.
This will add too much overhead. Let's do the reverse: whenever removeListener() is called, a new array is allocated.
There was a problem hiding this comment.
ok seems fair,
Im not sure how we'd implement that, because the array with the matched handlers is given to fastparallel in _do()
and the removeListener() is called within a matched handler that was called by fastparallel?
There was a problem hiding this comment.
Actually there is a better way: move the removal inside a setImmediate.
mqemitter.js
Outdated
| setImmediate(function () { | ||
| that._matcher.remove(topic, notify) | ||
| if (done) { | ||
| setImmediate(done) |
There was a problem hiding this comment.
No need for the additional setImmediate here
fixes #125