Promise.prototype.finally() never settles when the promise resolves after .finally() is attached #596

Description

A promise returned by .finally() on a workerpool Promise never settles, either resolved or rejected, if the original promise settles after .finally() was called. The callback passed to .finally() runs, but awaiting the result hangs forever. Every promise returned by pool.exec() is in that state, so await pool.exec(...).finally(...) never continues.

Reproduction

const workerpool = require('workerpool')

const main = async () => {
  const now = new workerpool.Promise((resolve) => resolve(7))
  console.log('now settled with', await now.finally(() => console.log('now: finally callback ran')))

  const later = new workerpool.Promise((resolve) => setTimeout(() => resolve(42), 10))
  const value = await later.finally(() => console.log('later: finally callback ran'))
  console.log('later settled with', value)
}

main()

Output:

now: finally callback ran
now settled with 7
later: finally callback ran

later settled with 42 is never logged. Node then exits with the await still pending, because nothing will ever settle it. Rejecting instead of resolving behaves the same way.

Cause

finally is implemented as:

Promise.prototype.finally = function (fn) {
  const me = this;

  const final = function() {
    return new Promise((resolve) => resolve())
      .then(fn)
      .then(() => me);
  };

  return this.then(final, final);
}

When me settles, _resolve runs _onSuccess.forEach(...), which calls final synchronously. Inside it, the synchronous chain reaches .then(() => me), and _then calls me.then(resolve, reject). At that point me's _process has not yet been replaced with the "already settled" version. That replacement only happens after the forEach. So the callback is pushed onto _onSuccess, and forEach never visits it because it stops at the array's original length. The promise returned by finally is never resolved.

In the already-settled case (now above), _process has already been replaced, so the callback is called immediately and everything works.

Possible fixes

• In _resolve / _reject, swap _process (and the _resolve / _reject no-ops) before running the queued callbacks, so a re-entrant then on the same promise runs immediately.

• Or implement finally without re-entering me:

  Promise.prototype.finally = function (fn) {
    return this.then(
      (result) => new Promise((resolve) => resolve(fn?.())).then(() => result),
      (error) => new Promise((resolve) => resolve(fn?.())).then(() => { throw error })
    );
  }

Workaround

Wrap the result in a native promise, at the cost of losing .cancel() / .timeout():

const result = await Promise.resolve(pool.exec('fn', [params])).finally(cleanup)