Bug 1631304 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I'm going to provide a lot of context here, because I think this machinery is not well-understood by people other than me. :-)

In the old days, anything that needed to use MozPromise needed to use AbstractThread, so AbstractThread began to propagate throughout the tree. We then switched MozPromise over to nsISerialEventTarget, which means that much of the AbstractThread usage still in the tree is vestigial. However, there is one piece of special functionality you still need AbstractThread for, which is TailDispatch [1].

The basic idea of tail dispatch is to delay the dispatching of all tasks from thread A to thread B until the end of the current task. The reason for this is the StateMirroring machinery [2]. The idea of state mirroring is to accumulate state updates in thread A and send them to thread B atomically once all the mutations have finished. This abstraction only holds if thread A doesn't share memory with thread B, and also doesn't have any asynchronous communication with thread B while state updates are accumulating. That is to say, if thread A is queuing up state updates to send to thread B later, but is intermittently firing runnables at thread B, the whole thing falls apart. Thread B might see a runnable that assumes the new state, but not have received the new state yet.

So the idea of tail dispatch is that it's global relationship between two threads - at least for the code using AbstractThread, which is why it's important to use AbstractThread consistently within a module. If thread A and thread B both support tail dispatch, then all runnables between them are queued until the end of the task, and dispatched atomically (with state updates going out first, and regular tasks going out second). So far this is all just queuing tasks, no arbitrary code execution.

However, in addition to StateMirroring, there is also a companion mechanism called StateWatching [3]. StateWatching is basically a quick-and-dirty version of reactive programming in Gecko, wherein you can tweak a bunch of values, and then run some dependent code like (i.e. StateDidChange()) once the dust has settled. StateWatching also piggy-backs off the TailDispatcher, but needs to run code immediately. So it has a mechanism called "Direct Tasks", which do that [4].

On the main thread, the TailDispatcher registers itself as a stable-state callback, to run at the very end of the current task in the event loop. This is conceptually what we want, but suboptimal for a few reasons:
* The concept of "stable state" is vestigial from the spec's perspective, and has been merged with the microtask infrastructure. TailDispatcher's current usage of it is very weird and not well-understood.
* Triggering script from within a stable-state callback is disallowed. This is sometimes happens accidentally, and triggers crashes (see bug 1443429).

Strictly speaking, the direct tasks don't _need_ to be coupled with the rest of the tail dispatch machinery. However, the AbstractThread abstraction allows StateWatching and StateMirroring to be used transparently across TaskQueues and XPCOM threads, which is a nice thing to preserve.

Given all the considerations, I think the right thing to do here is to run the tail dispatcher off the microtask queue. This will make it less weird, and allow Direct Tasks to trigger script (since microtasks are totally allowed to run script). The primary disadvantage is that the system might be less "settled" when the tail dispatcher fires, because MicroTasks run at the end of topmost script execution. So in this example:

```
void SomeRunnable() {
MutateSomeMirroredState();
TriggerSomeJS();
MutateSomeMoreMirroredState();
}
```

Today's Gecko would deliver the state updates as one atomic unit, whereas the MicroTask version would deliver two state updates. That could cause problems if the intermediate state is not coherent, but I think that's unlikely in practice. After all, if you're about to run JS, the modified state is probably going to be observable, so it's necessary anyway to make it coherent.

I prototyped this, and it appears to be green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf56740ae55cdbae7f89ee56000708677b05d59c

[1] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/AbstractThread.h#79
[2] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/StateMirroring.h
[3] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/StateWatching.h
[4] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/TaskDispatcher.h#44
[5] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/AbstractThread.cpp#80
I'm going to provide a lot of context here, because I think this machinery is not well-understood by people other than me. :-)

In the old days, anything that needed to use MozPromise needed to use AbstractThread, so AbstractThread began to propagate throughout the tree. We then switched MozPromise over to nsISerialEventTarget in bug 1366072, which means that much of the AbstractThread usage still in the tree is vestigial. However, there is one piece of special functionality you still need AbstractThread for, which is TailDispatch [1].

The basic idea of tail dispatch is to delay the dispatching of all tasks from thread A to thread B until the end of the current task. The reason for this is the StateMirroring machinery [2]. The idea of state mirroring is to accumulate state updates in thread A and send them to thread B atomically once all the mutations have finished. This abstraction only holds if thread A doesn't share memory with thread B, and also doesn't have any asynchronous communication with thread B while state updates are accumulating. That is to say, if thread A is queuing up state updates to send to thread B later, but is intermittently firing runnables at thread B, the whole thing falls apart. Thread B might see a runnable that assumes the new state, but not have received the new state yet.

So the idea of tail dispatch is that it's global relationship between two threads - at least for the code using AbstractThread, which is why it's important to use AbstractThread consistently within a module. If thread A and thread B both support tail dispatch, then all runnables between them are queued until the end of the task, and dispatched atomically (with state updates going out first, and regular tasks going out second). So far this is all just queuing tasks, no arbitrary code execution.

However, in addition to StateMirroring, there is also a companion mechanism called StateWatching [3]. StateWatching is basically a quick-and-dirty version of reactive programming in Gecko, wherein you can tweak a bunch of values, and then run some dependent code like (i.e. StateDidChange()) once the dust has settled. StateWatching also piggy-backs off the TailDispatcher, but needs to run code immediately. So it has a mechanism called "Direct Tasks", which do that [4].

On the main thread, the TailDispatcher registers itself as a stable-state callback, to run at the very end of the current task in the event loop. This is conceptually what we want, but suboptimal for a few reasons:
* The concept of "stable state" is vestigial from the spec's perspective, and has been merged with the microtask infrastructure. TailDispatcher's current usage of it is very weird and not well-understood.
* Triggering script from within a stable-state callback is disallowed. This is sometimes happens accidentally, and triggers crashes (see bug 1443429).

Strictly speaking, the direct tasks don't _need_ to be coupled with the rest of the tail dispatch machinery. However, the AbstractThread abstraction allows StateWatching and StateMirroring to be used transparently across TaskQueues and XPCOM threads, which is a nice thing to preserve.

Given all the considerations, I think the right thing to do here is to run the tail dispatcher off the microtask queue. This will make it less weird, and allow Direct Tasks to trigger script (since microtasks are totally allowed to run script). The primary disadvantage is that the system might be less "settled" when the tail dispatcher fires, because MicroTasks run at the end of topmost script execution. So in this example:

```
void SomeRunnable() {
MutateSomeMirroredState();
TriggerSomeJS();
MutateSomeMoreMirroredState();
}
```

Today's Gecko would deliver the state updates as one atomic unit, whereas the MicroTask version would deliver two state updates. That could cause problems if the intermediate state is not coherent, but I think that's unlikely in practice. After all, if you're about to run JS, the modified state is probably going to be observable, so it's necessary anyway to make it coherent.

I prototyped this, and it appears to be green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf56740ae55cdbae7f89ee56000708677b05d59c

[1] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/AbstractThread.h#79
[2] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/StateMirroring.h
[3] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/StateWatching.h
[4] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/TaskDispatcher.h#44
[5] https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/xpcom/threads/AbstractThread.cpp#80

Back to Bug 1631304 Comment 0