Timing bug inside semaphore for `isLocked()`

[CMCDragonkai]

I've discovered a timing bug involving withTimeout.

import { Mutex, withTimeout } from 'async-mutex';

async function main () {

  const lock = new Mutex();

  const release = await lock.acquire();

  const lock2 = withTimeout(lock, 100);

  try {
    await lock2.acquire();
  } catch (e) {
    console.log(e.message);
  }

  const lock3 = withTimeout(lock, 100);

  try {
    await lock3.acquire();
  } catch (e) {
    console.log(e.message);
  }

  release();

  console.log(lock.isLocked());

  setTimeout(() => {
    console.log(lock.isLocked());
  }, 0);

}

main();

Running with ts-node shows this:

timeout while waiting for mutex to become available
timeout while waiting for mutex to become available
true
false

We should expect that console.log(lock.isLocked()) to be false as soon as I've called release().

Note that this error goes away as soon as I remove the withTimeout attempts.

I've traced this to:

    Semaphore.prototype.isLocked = function () {
        return this._value <= 0;
    };

The _value property is still 0, and it appears the value isn't properly incremented until some ticks pass by. Which is why the setTimeout shows that the lock becomes unlocked afterwards.

So far, in order to fix this, I had to add a 0 ms sleep after my release calls.

[CMCDragonkai]

My node version:

[nix-shell:~/Projects/js-async-locks]$ node --version
v14.17.3

And ts-node version:

      "ts-node": "^10.4.0",

[CMCDragonkai]

Could this be related to #52?

[CMCDragonkai]

I found that yield by the event loop using queueMicrotask is more efficient to solve this problem.

[DirtyHairy]

Hm, I wouldn't classify this as a bug, there is no guarantee that the releasing the mutex will make it available immediately as there may be other waiters queued. This is precisely what happens here: withTimeout wraps acquire such that it will first acquire the mutex and then release it immediately if the timeout has passed.

[CMCDragonkai]

there is no guarantee that the releasing the mutex will make it available immediately as there may be other waiters queued

Well that's surprising to me. I thought that would be the case. Anyway I made the changes to my library abstraction to ensure this is the case by waiting for microtasks to finish during release. MatrixAI/js-async-locks#3

In more detail... given that there are no other waiters on the same lock, I expected that it would ensure that it would unlocked at that point.

[DirtyHairy]

But that's the point here, there are other waiters, as the withTimeout wrapper is still waiting for the mutex to be released, it's just the code that has called it that has moved on. Do you have a usecase where this is a real problem?

[CMCDragonkai]

Yea that makes sense, I looked at the withTimeout decorator, even after rejecting the promise with the timeout error, it would still end up waiting for the mutex to be released. Ultimately this is due to the lack of cancellable promises in native JS. So I'll close the issue for now.