Bug 1882019 Comment 2 Edit History

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

In any event, this does need to be addressed as-is, as noted there are a few lingering edge cases where we can be in this situation.

The basic situation here is:
- The MessagePort's invariants about won't hold until after we return from MessagePort::Initialize and the [owning MessageChannel calls UnshippedEntangle](https://searchfox.org/mozilla-central/rev/d87ac4f189d6c1ad068bc3d1cdf50d2f871028c2/dom/messagechannel/MessageChannel.cpp#75-76).
- [nsIGlobalObject::IsDying](https://searchfox.org/mozilla-central/rev/d87ac4f189d6c1ad068bc3d1cdf50d2f871028c2/dom/base/nsIGlobalObject.h#103) or [mozilla::GlobalTeardownObserver::CheckCurrentGlobalCorrectness](https://searchfox.org/mozilla-central/rev/d87ac4f189d6c1ad068bc3d1cdf50d2f871028c2/dom/base/GlobalTeardownObserver.cpp#50) are checks that we could also run earlier in the process and reliably would let us know we are headed for this eventual code path.  In particular, it's not possible for control runnables to be processed anywhere in the MessageChannel creation process at this time.  It's not particularly likely we would change that, as that would effectively constitute turning channel creation into a synchronous/blocking op.

Fix options:
1. Bail earlier with an IsDying check as noted above so this control flow path doesn't happen.  WorkerRef creation could then be made to more strongly assert success.
2. Add a new state machine state like `eStateInitializedUntangled` which indicates our intent to advance to `eStateUnshippedEntangled` when [mozilla::dom::MessagePort::UnshippedEntangle](https://searchfox.org/mozilla-central/rev/aff9f084975de6123b96aaa7c2edf31304c7c8c4/dom/messagechannel/MessagePort.cpp#226) is called.
3. Relax the assert.

I'm inclined to add a new state machine state for this case (2) in the interest of minimizing changes to the control flow and because it feels less prone to inadvertent breakage where it feels like the new dying check could be accidentally removed.  Also, the WorkerRef handling for the 1st case still essentially would need to be there in some form.  And also this is not a case we can really test.
In any event, this does need to be addressed as-is, as noted there are a few lingering edge cases where we can be in this situation.

The basic situation here is:
- The MessagePort's invariants about `eStateUnshippedEntangled` won't hold until after we return from MessagePort::Initialize and the [owning MessageChannel calls UnshippedEntangle](https://searchfox.org/mozilla-central/rev/d87ac4f189d6c1ad068bc3d1cdf50d2f871028c2/dom/messagechannel/MessageChannel.cpp#75-76).
- [nsIGlobalObject::IsDying](https://searchfox.org/mozilla-central/rev/d87ac4f189d6c1ad068bc3d1cdf50d2f871028c2/dom/base/nsIGlobalObject.h#103) or [mozilla::GlobalTeardownObserver::CheckCurrentGlobalCorrectness](https://searchfox.org/mozilla-central/rev/d87ac4f189d6c1ad068bc3d1cdf50d2f871028c2/dom/base/GlobalTeardownObserver.cpp#50) are checks that we could also run earlier in the process and reliably would let us know we are headed for this eventual code path.  In particular, it's not possible for control runnables to be processed anywhere in the MessageChannel creation process at this time.  It's not particularly likely we would change that, as that would effectively constitute turning channel creation into a synchronous/blocking op.

Fix options:
1. Bail earlier with an IsDying check as noted above so this control flow path doesn't happen.  WorkerRef creation could then be made to more strongly assert success.
2. Add a new state machine state like `eStateInitializedUntangled` which indicates our intent to advance to `eStateUnshippedEntangled` when [mozilla::dom::MessagePort::UnshippedEntangle](https://searchfox.org/mozilla-central/rev/aff9f084975de6123b96aaa7c2edf31304c7c8c4/dom/messagechannel/MessagePort.cpp#226) is called.
3. Relax the assert.

I'm inclined to add a new state machine state for this case (2) in the interest of minimizing changes to the control flow and because it feels less prone to inadvertent breakage where it feels like the new dying check could be accidentally removed.  Also, the WorkerRef handling for the 1st case still essentially would need to be there in some form.  And also this is not a case we can really test.

Back to Bug 1882019 Comment 2