1629526 - Crash in [@ MessageLoop::PostTask | mozilla::widget::WaylandDispatchDisplays]

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P3)

Description

[Gabriele Svelto [:gsvelto]]

This bug is for crash report bp-b1ddf323-2ae9-45f2-8339-f21130200412.

Top 10 frames of crashing thread:

0 libxul.so MessageLoop::PostTask ipc/chromium/src/base/message_loop.cc:346
1 libxul.so mozilla::widget::WaylandDispatchDisplays widget/gtk/nsWaylandDisplay.cpp:84
2 libxul.so nsAppShell::ProcessNextNativeEvent widget/gtk/nsAppShell.cpp:277
3 libxul.so <name omitted> widget/nsBaseAppShell.cpp:242
4 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1104
5 libxul.so nsThread::Shutdown xpcom/threads/nsThread.cpp:889
6 libxul.so nsThreadPool::Shutdown xpcom/threads/nsThreadPool.cpp:397
7 libxul.so mozilla::detail::RunnableMethodImpl<nsCOMPtr<nsIThreadPool>, nsresult  xpcom/threads/nsThreadUtils.h:1220
8 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1200
9 libxul.so mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:87

Only two crashes so far but consistent stacks across two different distros so it's probably a valid one.

Comment 1

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

I wonder what could be null there...

Comment 2

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

hm, maybe OOM where NewRunnableFunction() returns nullptr?

Comment 3

[Gabriele Svelto [:gsvelto]]

I'll try to inspect one of the minidumps tomorrow but I don't think it's an OOM because the machines involved on all three crashes had plenty of free memory. Since we're calling GetDispatcherThreadLoop() twice in a row could this be a race and during the second call mDispatcherThreadLoop is NULL?

Comment 4

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Yes, that's possible, thanks.
WaylandDispatchDisplays() uses gWaylandDisplaysMutex to avoid races so let's it also add to ::Shutdown() where mDispatcherThreadLoop is released (https://searchfox.org/mozilla-central/rev/2cd2d511c0d94a34fb7fa3b746f54170ee759e35/widget/gtk/nsWaylandDisplay.cpp#456).

Comment 5

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Attachment: Bug 1629526 [Wayland] Use mutex when clear mDispatcherThreadLoop, r?jhorak

Comment 6

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d658dfd1bbd5edb836f292b601058626231e0bc

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e94064c111d
[Wayland] Use mutex when clear mDispatcherThreadLoop, r=jhorak

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddcf3d470581
Fix whitespace lint on nsWaylandDisplay.cpp CLOSED TREE

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5e94064c111d
https://hg.mozilla.org/mozilla-central/rev/ddcf3d470581

Comment 10

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Hm, the patch here does not fix anything and also causes browser hang on quit on Wayland. Candidate to "the worst patch of year".

Comment 11

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Attachment: Bug 1629526 [Wayland] Add gWaylandThreadLoopMutex to guard thread loop shutdown, r?jhorak

• Add gWaylandThreadLoopMutex to guard thread loop shutdown
• Rename gWaylandDisplaysMutex to gWaylandDisplayArrayMutex to clarify it's mutex for gWaylandDisplays array access
• Add gWaylandDisplayArrayMutex lock at ReleaseDisplaysAtExit()
• Rename DispatcherThreadLoop names to ThreadLoop

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e8f6a6c6f4c
[Wayland] Add gWaylandThreadLoopMutex to guard thread loop shutdown, r=jhorak

Comment 13

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1e8f6a6c6f4c