Currently, lock.asyncHold() isn't specced to take any arguments. It just returns a promise, and you pass the code you want to operate under the lock to the .then() method. This is problematic - it requires promise subclassing, and has some unintuitive and probably unexpected behavior.
For example, this code doesn't lock anything at all:
const p = lock.asyncHold();
...never call .then()...
This is extremely different from lock.hold(cb);
, which immediately acquires the lock (and calls the cb with it held).
Similarly, this code doesn't lock anything either:
const p = lock.asyncHold();
const resultP = Promise.prototype.then.call(p, cb);
...because it uses the base Promise#then()
function, which doesn't have any special "acquire the lock before running the cb" semantics. This is why I say this functionality requires Promise subclassing - you're not using the base Promise#then function.
Here's another unintuitive/unexpected bit of behavior:
const p = lock.asyncHold();
p.then(cb1);
p.then(cb2);
This acquires the lock twice, once for cb1
and once for cb2
, from a single asyncHold()
call. This is very different from how .hold()
works, which acquires it only once.
The exact same arguments apply to asyncWait()
.
The core problem is that what you're looking for is a Task
, not a Promise
. Tasks
are single-cast (can only call .then()
on it once) and the Task
can respond to the .then()
being registered. (For example, they can delay actually doing some async work until something asks for it.) Passing the cb directly to asyncWait()
is emulating the functionality that a Task
would provide.
Assuming that it's necessary to subclass Promise (can't totally tell from this explanation why), why would that be a bad thing? Pulling out the 'then' method from the prototype doesn't seem likely. The fact that never using then() won't lock seems no different from what you propose with Task.
I'm also not sure why you think subclassing is necessary, but that could be just my ignorance. Would holding the lock around resolving the promise do the job? Why does then() need to do anything different? It just registers the callback, it does not invoke it (as I understand it). The way I understand the proposal, asyncHold() would imitate the process of asynchronously acquiring a lock right away, and once the lock is acquired (and you're back to the run loop) it resolves the promise with the lock held. This does mean that trying to then() more than once wouldn't do the right thing, so maybe explicitly passing a callback is better.
(All of this is a side issue to the main point of the post of course; clearly some async locking API could be made to work, even if the details are up for debate.)