rogierschouten/async-lock

Lock inside promise not actually locking?

roy7 opened this issue · 2 comments

roy7 commented

I may be doing something obviously wrong, I'm not sure. But I tried putting a lock around some code in a promise that was causing our server issues to be sure it was only done once at a time. However from my logs I see multiple clients ending up in the locked section simultaneously.

This is the function in question: https://github.com/gcp/leela-zero-server/blob/287ae0a852ff03a2cf0d528ee2c7c6981ec33c36/server.js#L182

The bulk of the Promise has the code wrapped in the lock.request(). But I can see extra clients are able to acquire the lock and start an unzip process even before the first unzip has completed.

Does the lock have to be outside the "return new Promise" above it? Do I need to "await" the lock.request() when called inside a Promise?

Any help appreciated. Thanks!

Hi @roy7, two remarks:

  • In half the branches you don't return any value from the callback, causing it to return immediately and release the lock
  • Instead of returning the new promise, you should return (or return await) the lock.request

When submitting an issue on Github (or Stackoverflow or anywhere else for that matter), don't expect other people to look at 60+ lines of your code. Next time, create a minimalistic example that exhibits the problem. It might even help you solve the issue yourself, but if not, it will make it much more likely that someone will look at it in their spare time.

roy7 commented

No worries, apologies for not just stripping out all the useless code and only giving you the shell. :)

Thanks for the quick reply though!