1882019 - Crash in [@ mozilla::dom::MessagePort::CloseInternal]

Status: RESOLVED FIXED

(Core :: DOM: postMessage, defect, P2)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/0f0b2a77-d902-4b0c-a500-698330240217

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(mUnshippedEntangledPort)

Top 10 frames of crashing thread:

0  libxul.so  mozilla::dom::MessagePort::CloseInternal  dom/messagechannel/MessagePort.cpp:463
1  libxul.so  mozilla::dom::MessagePort::CloseForced  dom/messagechannel/MessagePort.cpp:449
1  libxul.so  mozilla::dom::MessagePort::Initialize  dom/messagechannel/MessagePort.cpp:269
2  libxul.so  mozilla::dom::MessagePort::Create  dom/messagechannel/MessagePort.cpp:208
3  libxul.so  mozilla::dom::MessageChannel::Constructor  dom/messagechannel/MessageChannel.cpp:65
4  libxul.so  mozilla::dom::MessageChannel::Constructor  dom/messagechannel/MessageChannel.cpp:43
4  libxul.so  mozilla::dom::MessageChannel_Binding::_constructor  dom/bindings/MessageChannelBinding.cpp:235
5  libxul.so  CallJSNative  js/src/vm/Interpreter.cpp:480
5  libxul.so  CallJSNativeConstructor  js/src/vm/Interpreter.cpp:496
5  libxul.so  InternalConstruct  js/src/vm/Interpreter.cpp:721

Low-volume because the assertion is only enabled on beta and nightly.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Hm, these all show the worker script loader seems to still be evaluating code after having been notified of shutdown. I'm inclined to suspect the involvement of bug 1837302 given the rarity of these.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

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.
• nsIGlobalObject::IsDying or mozilla::GlobalTeardownObserver::CheckCurrentGlobalCorrectness 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 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.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Bug 1882019 - Handle failure to create a WorkerRef better. r=#dom-workers!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c256ec57d1b
Handle failure to create a WorkerRef better. r=dom-worker-reviewers,smaug

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/4c256ec57d1b