@triblondon to write some notes here.

I should note that the IDL link given is obsolete; IDL fragments have been rolled into the proto-spec.

Thanks for the attention!

On Jan 16, 2018, at 8:47 AM, Daniel Appelquist notifications@github.com wrote:

@triblondon to write some notes here.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

This seems like a cool feature. I have some questions:

I'm wondering why the callback based API that also seems to incorporate promises was chosen. Explicit release is documented as an alternate API that was considered but there isn't much on why you chose the callback API instead. I'd instinctively want this (explicit release):

const lock = await navigator.locks.acquire('my-lock');
await doSomethingAsync();
await lock.release();

It seems odd to me that the promise returned by acquire() actually doesn't resolve when the lock is acquired but rather when it is released. I also wonder if people will often forget to await on the async operation inside the callback:

// Appears to work but actually releases lock before async operation starts
await navigator.locks.acquire('my-lock', () => doSomethingAsync());

The impact of this design decision seems to be that on average locks will err on the side of releasing too early rather than too late. Given that they are origin-scoped, and therefore late-releasing locks can only impact the author's own applications, I would think we could provide the more explicit version.

I can imagine scenarios in which I might want to hold multiple locks, but in unconnected non-nested operations, which happen to overlap. That seems a lot easier to reason about with explicit locks.

If we are to have the callback shaped API, which resolves upon release, perhaps acquire is the wrong word - I'd consider wrap, enclose, lock or lockFor.

Have you considered a built in timeout or do we consider AbortController to be the right way to expose the ability to do that now?

I'm wondering why the callback based API that also seems to incorporate promises was chosen.

The best places that captures the debate is at: https://github.com/inexorabletash/web-locks/issues/9 - the design started with explicit release, but evolved away in response to feedback from @domenic, @jakearchibald and others.

The callback+promises approach sounds weird in the abstract and I was skeptical, but the resulting code ends up looking surprisingly clean. Agreed that you need to remember the await and async sprinkles. But as noted in that bug thread, writing correct code with other forms is convoluted and the language doesn't help you get it right. So... a trade off that (we felt) is somewhat closer closer to a "pit of success".

It does make the API awkward when needing explicit lifetime control; you end up recreating the explicit API with explicit promise hi-jinx. (e.g. this shows up in tests quite a bit)

Have you considered a built in timeout or do we consider AbortController to be the right way to expose the ability to do that now?

Definitely considered — it was an explicit option in initial API sketches. Having it built in allows for the lock manager to run the timer, which is an advantage if e.g. a page is janking. But AbortController appears to be a generalization, so exposing just that for now. We look forward to developer feedback once it's implemented.

Worth noting also that this callback mechanism makes web locks inconsistent with all the other numerous lock related APIs. We've established four distinct patterns so far for acquiring a lock on something. Do we really need a fifth?

I think web locks are very different from the type of locks listed in those comments; it's unfortunate that sometimes we use the same English word for two separate concepts, but I think that's what's happening here. In particular those are about locking other people away from using a resource (keyboard, pointer) or locking something in a certain position (wake, orientation). Web locks are about the CS concept of a lock/mutex.

On the flip side, this actually seems a bit different from a lock/mutex, given that it's promise-based and thus doesn't block the execution stack (except for async functions). (That also, if I'm understanding correctly, greatly reduces the consequences of deadlocks, although the locks certainly can still be deadlocked.) @travisleithead wondered if perhaps this should be named in terms of transactions rather than locks.

I also think it's still similar enough in concept that the API surface shouldn't be unnecessarily different.

The TAG just discussed this at our London F2F meeting and concluded that:

• The callback approach has some problems in its current form:
  • Forgetting to use an async callback is a serious footgun. It will appear to work and you may not discover the bug for some time. Throwing an error if the callback doesn't return a promise might mitigate this.
  • The term acquire is inappropriate to what the method actually does. It looks more like a transaction. A rename could mitigate this.
• We also looked at the two alternative API proposals
  • The waitUntil proposal seems like a misuse of the existing definition of that method in the serviceworker context, since the return value from requestLock is not an event.
  • It's confusing that the waitUntil method doesn't block, meaning that the scope in which the lock is defined becomes garbage collectable before the lock releases
  • Explicit release may have a higher likelihood of accidental long lived locks and mistakes in error handling code.
• We conceived of a fourth option, using extendable events in a manner more consistent with their use in service workers:

const lock = new WebLock('lock-name');
lock.on('acquire', e => {
  e.waitUntil(doSomethingAsync());
});

• We recognise that Web Locks are not the same kind of lock as keyboard locks and wake locks, but they share the name, so the API shape being different is unexpected. We see that this feature was previously called requestFlag and maybe that or another name should be considered.

Thanks folks! I'll chew this over and work through it with others involved in the design.

I really appreciate the attention.

We've posted further feedback on their issue https://github.com/inexorabletash/web-locks/issues/35, and that about wraps this up.