-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Queueing a task should acknowledge its race #6475
Comments
@jakearchibald might have thoughts on this. |
Yeah, I think it's fine to use something like "atomically" and note the race in prose. We could try and spec it fully with parallel queues, but I don't see much benefit. |
Parallel queues don't actually help in this case, because the whole point of "queue a task" is to synchronize across threads. ... That did make me notice that parallel queues themselves could use some "atomically" in the operations that enqueue and dequeue. |
I was assuming you could put something in the parallel queue then wait for it on the main thread, but I guess that isn't allowed. |
I think what matters is that from the point of view of the parallel steps enqueuing the task, tasks are run in the order in which they were enqueued from those same parallel steps, as opposed to "synchronize across threads", since no ordering of queuing is assumed when tasks are queued from different threads(read: parallel steps).
It is also used from steps running on the event-loop. See for example https://html.spec.whatwg.org/multipage/#the-object-element:queue-a-task
It's true that a task queue is a infra set, and it should also be noted that this doesn't make it a specific JavaScript realm, global, or environment settings object, which makes manipulating it directly from parallel steps acceptable(see https://html.spec.whatwg.org/multipage/l#event-loop-for-spec-authors).
What does "Atomically" mean in that context, since you're not trying to synchronize ordering of tasks across threads? This goes both for parallel queues and "normal" task queues(since an event-loop is really a kind of parallel queue from the perspective of steps running in-parallel of it). For example, if you had two different parallel steps, and you needed them to queue tasks in a specific order back on one event-loop, then you'd have to use a shared parallel queue, on which both parallel steps would enqueue steps(in an unspecified order), and that parallel queue could then determine the "order" in which the corresponding tasks would be queued on a task source back on an event-loop(for example by waiting for both, and then queuing tasks "in order"). In this case the ordering would be guaranteed by the logic inside this parallel queue, used to synchronize the outcome of two separate set of parallel steps. On the other hand, if those two set of parallel steps were to enqueue their tasks independently, and yet "atomically", directly on a task queue of an event-loop, you'd still have no guarantee of ordering(just like you have no guarantee of ordering when it comes to those two parallel steps enqueuing steps on this third parallel queue, however you can have this parallel queue wait for both steps and then do something in a certain order). |
I took it as just literally synchronizing access to the infra set across threads (no ordering guarantees across in-parallel contexts). A naive implementation could simply have multiple threads (in-parallel contexts) accessing the same underlying task queue (i.e., main thread's event loop) at the same time leading to phantom writes or otherwise corrupted reads due to a lack of thread safety. Maybe prescribing the antidote to this (i.e., synchronizing access to the set) is an implementation detail, but at minimum adding a note about this somewhere seems harmless enough. |
https://html.spec.whatwg.org/multipage/webappapis.html#queue-a-task is meant to be called from 'in parallel', but it modifies a task queue, which is a normal Infra set, and that task queue is then used from the main thread without synchronization. We should do something to avoid normalizing that kind of race.
It might be fine to just say "Atomically append" task to queue, and matching wording when tasks are dequeued.
The text was updated successfully, but these errors were encountered: