Editorial: Queue a task to resolve/reject promise or when fire an event.

[monica-ch]

This is for addressing #1740.

---

Preview | Diff

[yoshisatoyanagisawa]

In #1744, I was asked to introduce the parallel queue. I am revisiting it to see if the case also need to be applied here or not.

[yoshisatoyanagisawa]

I started to wonder whether we need [=in parallel=] if almost all sub steps under [=in parallel=] is enqueued.
Considering how I code the algorithm, I would write base::Callback() of the sub steps to be queued, which might match the behavior that simply enqueue the sub steps to the parallel queue.
What do you think?

I am not so familiar with how specs are written to realize parallel jobs.
Let me ask @domenic for advice.

[domenic]

This diff looks locally reasonable to me. However, I am not caught up on the discussions about parallel queues vs. using the generic "in parallel", so I cannot say which is is best here. Maybe merging this first is good and then doing some overall work on using parallel queues later is the right approach.

---

To try to answer the more generic questions:

Ideally, specifications are written to match implementations:

• There are completely separate threads, which mostly do not touch.
  • There is one event loop thread per agent
  • There is a thread for each specific parallel queue
  • Every invocation of the generic "in parallel" can be thought of as spawning a new separate thread
• In particular, it's important to be very clear what objects and data structures are touchable from each spec thread
  • Objects with a strong connection to JavaScript, like promises or Web IDL interface objects, must only be touched from their corresponding event loop thread.
    • But see below for how we don't always perfectly follow this advice.
  • Simple primitive values like booleans or strings, or more complex "primitive-ish" values like sets of integers or Web IDL dictionaries/Infra maps whose values are only primitives, can be touched from any thread. However, you need to take care to avoid race conditions.
    • We usually pass these objects across threads implicitly, with no clear indication that we're doing so. E.g., by having "in parallel" steps close over a variable that was introduced in the event loop portion of an algorithm.
    • This is often fine because 80% of the time, these values just make a one-way trip from the event loop (where they came from the web developer as method arguments or similar) to the in parallel thread (where they get processed).
    • Sometimes we need to do something more complex. Then it's best to add informative notes that are explicit about how you're avoiding race conditions. E.g. see this step in a spec I recently wrote.
  • Some objects represent "browser process" data structures, and as such you want to ensure you only touch them from "in parallel", or from a specific parallel queue.
    • Usually a specific parallel queue is better, but probably there are cases where it's unnecessary.
    • For example, the user agent's list of all shared worker global scopes is only accessed from the shared worker manager parallel queue. (But see below on SharedWorkerGlobalScope.)
• Sometimes there is an unfortunate mixing up between JavaScript objects and their "browser process" counterparts, which causes specs to end up touching those JS objects across multiple threads, even though this violates the above rules.
  • For example, there is a lot of information stored on the Document object in specifications, like URL or origin, which needs to be accessed from tons of different threads.
  • Or, the example linked above about shared worker manager is specified to look at the "list of all SharedWorkerGlobalScope objects", which violates the above rules, because SharedWorkerGlobalScope is a JavaScript object that should only exist on the event loop.
  • A fully rigorous spec for this would match implementations better. For example, it would have separate spec objects for Document and "browser-process document-related info". It would then need to specify the exact way in which this information is synchronized across the two representations. However, this is a lot of work, for unclear benefit.
  • An example of a spec which does this more correctly is Fetch, which separates requests from Requests.
    • However, the synchronization is kind of magical there, instead of explicit, so it's possible there are spec bugs or mismatches with the implementation, because the "requests" are accessed from multiple threads without explicit transfer algorithms, or explicit guards against racy multiple-writes.
    • But this is probably our best model today of how to separate things, while not making the spec excessively complicated.
    • The service worker spec seems to mostly follow this model.

---

How does this apply to the service worker spec, and to this PR?

Well, I think the main thing is to identify from which thread you intend "service worker registration" objects to be accessed. I get the impression it should be in parallel, not on an event loop thread. So that's why it's worth keeping something parallel-ish in getRegistration(), whether it's "in parallel" or a specific parallel queue. And then, you need to queue a task to resolve the promise. So that's why this PR looks reasonable to me.

Note that in this PR there is some thread crossing, where registration is implicitly passed from "in parallel" (where it was created in step 8.1) to the event loop thread (in step 8.2.1 and 8.2.2). This is a bit sketchy, but mostly OK. In practice, what's probably happening here is a copy and snapshot: when "get the service worker registration object" consults registration's installing worker in step 2.6, it's probably not doing some sort of cross-thread lookup at the actual registration object, but instead looking at some key that was passed across threads in getRegistration() step 8.2.

A more fully rigorous version of this whole setup would have explicitly-obvious primitive keys (like string UUIDs, or something) that allow us to correspond between browser-process "service worker"/"service worker registration" objects and event loop ServiceWorker/ServiceWorkerRegistration objects. Then algorithms like getRegistration() would pass across those keys in a way that makes it clear we're taking a snapshot. Then back on the event loop thread we'd look up the right ServiceWorker/ServiceWorkerRegistration objects and use those to construct the return value we resolve the promise with. But that's a lot of work, for unclear gain. The current model of just using the identity of the "service worker"/"service worker registration" objects, instead of explicit primitive keys, and understanding that it's implicitly snapshotted when crossing thread boundaries, is reasonable enough.

If I were to suggest the most useful improvements to improving the spec's cross-thread hygiene, here would be my priority list:

1. PRs like this, which clarify which task source is used to resolve promises.
2. Notate every algorithm in Appendix A with whether they run "in parallel" or on the event loop. This might uncover some problems. (I've been doing this recently, e.g. here and here.)
3. Audit all "in parallel" algorithms to ensure they aren't touching JS objects.
4. Use parallel queues appropriately to avoid race conditions from separate "in parallel" situations.
5. Figure out the relationship between parallel queues and the spec's existing idea of "jobs". (Maybe @jakearchibald can comment on this, as I think he might have invented both concepts.) Potentially rebase "jobs" on parallel queues?
6. Consider some mechanism for being more clear about how and when things cross threads. (This is generally not a solved problem in the spec ecosystem. Above I discussed one method, of using explicitly primitive keys, but maybe there is something more lightweight.)

I think it is totally fine if only item 1 on this list gets done. Items like 2-4 can just be things you keep in mind when you write new spec text.

[yoshisatoyanagisawa]

If this is for protecting [=navigation preload enabled flag=] to be updated in parallel, I guess we also need the code to read [=navigation preload enabled flag=] in Handle Fetch to be executed with queue?

[monica-ch]

In Handle Fetch algorithm I see [=navigation preload enabled flag=] is being used to check and also don't see they are running the steps are in [=in parallel=]

[yoshisatoyanagisawa]

Why not queue in Line 1391, and run invocation and resolve in its substeps?

[yoshisatoyanagisawa]

Also, is it guaranteed that client state wont change during the parallel execution, right?

[yoshisatoyanagisawa]

For Client API, it might affect not only service worker but also clients. Do we need a parallel queue that covers both instead of the event loop for ServiceWorker itself?

[monica-ch]

Updated the queue to resolve in substeps.

@yoshisatoyanagisawa Can you please elaborate on your third comment in this thread please

For Client API, it might affect not only service worker but also clients. Do we need a parallel queue that covers both instead of the event loop for ServiceWorker itself?

[yoshisatoyanagisawa]

Considering the possible modification of the map, I guess we want to execute followings inside a parallel queue.
It might be good to have a dedicated parallel queue for the Cache API.

[monica-ch]

Can you please also elaborate your suggestion