Bug 1798773 Comment 15 Edit History

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

I looked a bit into [the pernosco session](https://pernos.co/debug/OO1svbL1Qa7rBL7G0OUjVQ/index.html) and I wondered if in general we should try to handle a `MessageEventRunnable` when a worker is already set into `Canceling` state. We currently [check this only if ParentThreadUnchangedBusyCount](https://searchfox.org/mozilla-central/rev/6f77213807eb5359c8afe458ac5628f973e92a25/dom/workers/MessageEventRunnable.cpp#111-116) and that is not the case here as we arrive on the target worker thread here.
So IIUC we are handling the `onmessage` even though we are `Canceling`and that makes all the JS inside the `onmessage` block run as one microtask inside the same single runnable, with all that can happen from that.

In a nutshell: after we do "postMessage to worker" (happening before we cancel) there seems to be a general race possible between "cancel the worker" and "handle the onmessage on the worker". And when the `Canceling` wins, the onmessage handler will do things that are not wanted/expected anymore.

I did some [try pushes like this](https://treeherder.mozilla.org/jobs?revision=99c33fac54f42fd464191ed0a695cd6ca175e3e0&repo=try), and though there are quite some (known) intermittents I was not able to pinpoint one where the new canceling was actually triggered.

Unfortunately I was not able to trigger the testcase locally. Tyson would you mind doing a test with the [builds from this try](https://treeherder.mozilla.org/jobs?revision=99c33fac54f42fd464191ed0a695cd6ca175e3e0&repo=try)? 

In case it helps I am far from sure if
1) we should actually do what the try push proposes and if `Cancel()` the `MessageEventRunnable` is the right thing to do, though it [might seem wanted that we avoid any worker's Javascript execution once canceled](https://html.spec.whatwg.org/multipage/workers.html#terminate-a-worker)
2) if there is something bogus going on with the `FileSystemManager` handling of promises itself that might bite us in other occasions
I looked a bit into [the pernosco session](https://pernos.co/debug/OO1svbL1Qa7rBL7G0OUjVQ/index.html) and I wondered if in general we should try to handle a `MessageEventRunnable` when a worker is already set into `Canceling` state. We currently [check this only if ParentThreadUnchangedBusyCount](https://searchfox.org/mozilla-central/rev/6f77213807eb5359c8afe458ac5628f973e92a25/dom/workers/MessageEventRunnable.cpp#111-116) and that is not the case here as we arrive on the target worker thread.
So IIUC we are handling the `onmessage` even though we are `Canceling`and that makes all the JS inside the `onmessage` block run as one microtask inside the same single runnable, with all that can happen from that.

In a nutshell: after we do "postMessage to worker" (happening before we cancel) there seems to be a general race possible between "cancel the worker" and "handle the onmessage on the worker". And when the `Canceling` wins, the onmessage handler will do things that are not wanted/expected anymore.

I did some [try pushes like this](https://treeherder.mozilla.org/jobs?revision=99c33fac54f42fd464191ed0a695cd6ca175e3e0&repo=try), and though there are quite some (known) intermittents I was not able to pinpoint one where the new canceling was actually triggered.

Unfortunately I was not able to trigger the testcase locally. Tyson would you mind doing a test with the [builds from this try](https://treeherder.mozilla.org/jobs?revision=99c33fac54f42fd464191ed0a695cd6ca175e3e0&repo=try)? 

In case it helps I am far from sure if
1) we should actually do what the try push proposes and if `Cancel()` the `MessageEventRunnable` is the right thing to do, though it [might seem wanted that we avoid any worker's Javascript execution once canceled](https://html.spec.whatwg.org/multipage/workers.html#terminate-a-worker)
2) if there is something bogus going on with the `FileSystemManager` handling of promises itself that might bite us in other occasions
I looked a bit into [the pernosco session](https://pernos.co/debug/OO1svbL1Qa7rBL7G0OUjVQ/index.html) and I wondered if in general we should try to handle a `MessageEventRunnable` when a worker is already set into `Canceling` state. We currently [check this only if ParentThreadUnchangedBusyCount](https://searchfox.org/mozilla-central/rev/6f77213807eb5359c8afe458ac5628f973e92a25/dom/workers/MessageEventRunnable.cpp#111-116) and that is not the case here as we arrive on the target worker thread.
So IIUC we are handling the `onmessage` even though we are `Canceling`and that makes all the JS inside the `onmessage` block run as one microtask inside the same single runnable, with all that can happen from that.

In a nutshell: after we do "postMessage to worker" (happening before we cancel) there seems to be a general race possible between "cancel the worker" and "handle the onmessage on the worker". And when the `Canceling` wins, the onmessage handler will do things that are not wanted/expected anymore.

I did some [try pushes like this](https://treeherder.mozilla.org/jobs?revision=99c33fac54f42fd464191ed0a695cd6ca175e3e0&repo=try), and though there are quite some (known) intermittents I was not able to pinpoint one where the new canceling was actually triggered.

Unfortunately I was not able to trigger the testcase locally. Tyson, would you mind doing a test with the [builds from this try](https://treeherder.mozilla.org/jobs?revision=99c33fac54f42fd464191ed0a695cd6ca175e3e0&repo=try)? 

In case it helps I am far from sure if
1) we should actually do what the try push proposes and if `Cancel()` the `MessageEventRunnable` is the right thing to do, though it [might seem wanted that we avoid any worker's Javascript execution once canceled](https://html.spec.whatwg.org/multipage/workers.html#terminate-a-worker)
2) if there is something bogus going on with the `FileSystemManager` handling of promises itself that might bite us in other occasions

Back to Bug 1798773 Comment 15