1514733 - Crash in mozilla::ipc::MessageChannel::CxxStackFrame::CxxStackFrame | mozilla::ipc::MessageChannel::Send | mozilla::dom::PRemoteWorkerParent::SendExecOp

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Gabriele Svelto [:gsvelto]]

This bug was filed from the Socorro interface and is
report bp-e919cc76-1491-4467-97c4-a008f0181216.
=============================================================

Top 10 frames of crashing thread:

0 libxul.so mozilla::ipc::MessageChannel::CxxStackFrame::CxxStackFrame ipc/glue/MessageChannel.h:538
1 libxul.so mozilla::ipc::MessageChannel::Send ipc/glue/MessageChannel.cpp:979
2 libxul.so mozilla::dom::PRemoteWorkerParent::SendExecOp ipc/ipdl/PRemoteWorkerParent.cpp:90
3 libxul.so mozilla::dom::RemoteWorkerController::RemoveWindowID dom/workers/remoteworkers/RemoteWorkerController.cpp:201
4 libxul.so mozilla::dom::SharedWorkerManager::RemoveActor dom/workers/sharedworkers/SharedWorkerManager.cpp:104
5 libxul.so mozilla::dom::SharedWorkerParent::ActorDestroy dom/workers/sharedworkers/SharedWorkerParent.cpp:35
6 libxul.so mozilla::ipc::PBackgroundParent::DestroySubtree ipc/ipdl/PBackgroundParent.cpp:4060
7 libxul.so mozilla::ipc::PBackgroundParent::OnChannelError ipc/ipdl/PBackgroundParent.cpp:3730
8 libxul.so mozilla::detail::RunnableMethodImpl<mozilla::ipc::MessageChannel*, void  xpcom/threads/nsThreadUtils.h:1106
9 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1157

=============================================================

We're hitting this assertion:

https://hg.mozilla.org/mozilla-central/annotate/5b0b8a39d09e49efe283b94deba6853e223edad3/ipc/glue/MessageChannel.h#l538

MOZ_RELEASE_ASSERT(mWorkerThread == GetCurrentVirtualThread()) (not on worker thread!)

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

This is weird and might actually be an IPC bug (vs. an error in how IPC is used by, in this case, workers): PRemoteWorker and PSharedWorker are both managed by PBackground, and all the PBackgroundParent instances should be bound to the same thread.  So even if the PBackground being torn down isn't for the same content process thread as the PRemoteWorker at the end of the stack, that assertion wouldn't fire.  Unless I'm missing something.

Bug 1438945 might be related; it's about shared workers, and the first build with its patches is 20181120100045, which is also the first build where this crash is reported.

Comment 2

[David Burns :automatedtester]

Just hit this when restarting nightly with updates and lost all of my open tabs that were not pinned

Comment 3

[Andrew Overholt [:overholt]]

David, do you have a crash report?

(I'm going to move this to IPC based on comment 2.)

Comment 4

[David Burns :automatedtester]

https://crash-stats.mozilla.org/report/index/94c57a04-467d-431b-af06-a37c50190326 is the crash report

Comment 5

[Neha Kochar [:neha]]

Baku, is this something you can look into as this is related to the shared workers?

Comment 6

[Andrea Marchesini [:baku]]

Attachment: Bug 1514733 - RemoteWorkerController should be notified when the RemoteWorkerParent actor is dismissed, r?asuth

Comment 7

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

(Whoops. I had an in-progress analysis that didn't get submitted to this bug and instead was aggregated into my review comments. Bugzilla's localStorage caching restored the in-progress comment and I accidentally submitted it. Editing the comment here to provide this explanation and then I'll mark it obsolete or what not.)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5954214372b7
RemoteWorkerController should be notified when the RemoteWorkerParent actor is dismissed, r=asuth

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/5954214372b7

Comment 10

[Pascal Chevrel:pascalc]

Baku, do you want to uplift to beta?

Comment 11

[Andrea Marchesini [:baku]]

Comment on attachment 9057324 [details]
Bug 1514733 - RemoteWorkerController should be notified when the RemoteWorkerParent actor is dismissed, r?asuth

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: SharedWorkers in remote processes
• User impact if declined: A crash can occur during the shutdown
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The problem is that, when the parent actor is dismissed, we don't inform the manager.
  The fix is trivial, but we cannot write tests because this is a race condition and it's not easy to reproduce it consistently.
• String changes made/needed:

Comment 12

[Pascal Chevrel:pascalc]

Comment on attachment 9057324 [details]
Bug 1514733 - RemoteWorkerController should be notified when the RemoteWorkerParent actor is dismissed, r?asuth

Low risk fix for a medium crash on beta, uplift approved, thanks.

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fe71267f45ce