mcollina / fastq

Fast, in memory work queue
ISC License
932 stars 47 forks source link

the implementation of `drained()` api potentially can cause `maximum stack exceeded` error #86

Open normanzb opened 2 weeks ago

normanzb commented 2 weeks ago

the drained() function wraps existing drain() callback and replace it with the new one, and it only got reset back to noop when killed, imagine if somebody call the await drained() a few thousands times in a for-loop (like a long running process), it will create a huge stack and blow when somebody calls drain() later. that will stop the promise from being resolved or even worse when somebody calls killAndDrain() it will crash.

should use an array instead or use a 3rd party event lib or just cache the drained promise, or at least reset it everytime drained (only mitigate the issue).

  function drained () {
    if (queue.idle()) {
      return new Promise(function (resolve) {
        resolve()
      })
    }

    var previousDrain = queue.drain

    var p = new Promise(function (resolve) {
      queue.drain = function () {
        previousDrain()
        resolve()
      }
    })

    return p
  }
}