1624745 - Fedora 32 Crash in [@ <name omitted> | moz_container_get_wl_surface]

Status: RESOLVED FIXED

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

Description

[Gabriele Svelto [:gsvelto]]

This bug is for crash report bp-535f302e-b58a-4783-99de-6fb8b0200324.

Top 10 frames of crashing thread:

0 libwayland-client.so.0.3.0 <name omitted> src/wayland-client.c:793
1 libxul.so moz_container_get_wl_surface /builddir/build/BUILD/firefox-74.0/widget/gtk/mozcontainer.cpp:620
2 libxul.so nsWindow::GetWaylandSurface /builddir/build/BUILD/firefox-74.0/widget/gtk/nsWindow.cpp:7512
3 libxul.so mozilla::widget::WindowSurfaceWayland::CommitWaylandBuffer /builddir/build/BUILD/firefox-74.0/widget/gtk/WindowSurfaceWayland.cpp:1067
4 libxul.so mozilla::layers::BasicCompositor::EndRemoteDrawing /builddir/build/BUILD/firefox-74.0/gfx/layers/basic/BasicCompositor.cpp:1167
5 libxul.so mozilla::layers::BasicCompositor::EndFrame /builddir/build/BUILD/firefox-74.0/gfx/layers/basic/BasicCompositor.cpp:1100
6 libxul.so mozilla::layers::LayerManagerComposite::Render /builddir/build/BUILD/firefox-74.0/gfx/layers/composite/LayerManagerComposite.cpp:1302
7 libxul.so mozilla::layers::LayerManagerComposite::UpdateAndRender /builddir/build/BUILD/firefox-74.0/gfx/layers/composite/LayerManagerComposite.cpp:645
8 libxul.so mozilla::layers::LayerManagerComposite::EndTransaction /builddir/build/BUILD/firefox-74.0/gfx/layers/composite/LayerManagerComposite.cpp:534
9 libxul.so mozilla::layers::CompositorBridgeParent::CompositeToTarget /builddir/build/BUILD/firefox-74.0/gfx/layers/ipc/CompositorBridgeParent.cpp:1047

Looks like a NULL-pointer dereference in Wayland code. This is a crash specific to Fedora 32.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

Seems to be here:

container->subsurface = wl_subcompositor_get_subsurface(
waylandDisplay->GetSubcompositor(), container->surface, parent_surface);
GdkWindow* window = gtk_widget_get_window(GTK_WIDGET(container));
gint x, y;
gdk_window_get_position(window, &x, &y);
moz_container_move(container, x, y);

wl_subsurface_set_desync(container->subsurface); <<<<<<

so perhaps wl_subcompositor_get_subsurface() returns null.

Comment 2

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1624745 [Wayland] Handle error states when wayland surfaces are created, r?jhorak

Comment 3

[Martin Stránský [:stransky] (ni? me)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e39a84cf8e40d2fb806f01828223c268ee50a51

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5081d4443a9
[Wayland] Handle error states when wayland surfaces are created, r=jhorak

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/a5081d4443a9

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Does this need a Beta approval request?

Comment 7

[Martin Stránský [:stransky] (ni? me)]

I won't hurt to uplift to beta, Thanks.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 9135619 [details]
Bug 1624745 [Wayland] Handle error states when wayland surfaces are created, r?jhorak

Beta/Release Uplift Approval Request

• User impact if declined: May cause browser crash on Wayland when compositor fails to create a subsurface - perhaps it's called too early.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• 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): Linux/Wayland only.
• String changes made/needed: none

Comment 9

[Julien Cristau [:jcristau] (back April 22)]

Please don't use "linix/wayland only" as justification for how low the risk is. If the benefit is linux/wayland only, then the risk being linux/wayland only doesn't offset anything, IMO.

Comment 10

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9135619 [details]
Bug 1624745 [Wayland] Handle error states when wayland surfaces are created, r?jhorak

approved for 75rc1

Comment 11

[Darkspirit]

(In reply to Julien Cristau [:jcristau] from comment #9)

Please don't use "linix/wayland only" as justification for how low the risk is. If the benefit is linux/wayland only, then the risk being linux/wayland only doesn't offset anything, IMO.

Wayland backend is opt-in by env var MOZ_ENABLE_WAYLAND=1 or GDK_BACKEND=wayland (bug 1543600), otherwise X11 (XWayland) is used on Wayland. The risk of using Firefox' Wayland backend is higher than getting any fixes for it. On KDE, for example, Wayland backend often completely freezes Firefox which is getting fixed in KDE Plasma (bug 1609115).

Comment 12

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-release/rev/af7cca0a9111

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 16

[Gabriele Svelto [:gsvelto]]

We're still seeing crashes with a similar stack in recent builds, this one is from Debian:

https://crash-stats.mozilla.org/report/index/1bbc19f2-b9c2-4dc0-ab64-9d7860200425

And this one from Arch:

https://crash-stats.mozilla.org/report/index/640ada62-36ce-41f2-be23-5fcbc0200426

Does this look like a different issue?

Comment 17

[Martin Stránský [:stransky] (ni? me)]

(In reply to Gabriele Svelto [:gsvelto] from comment #16)

We're still seeing crashes with a similar stack in recent builds, this one is from Debian:

https://crash-stats.mozilla.org/report/index/1bbc19f2-b9c2-4dc0-ab64-9d7860200425

And this one from Arch:

https://crash-stats.mozilla.org/report/index/640ada62-36ce-41f2-be23-5fcbc0200426

Does this look like a different issue?

It looks like Bug 1628583 which was fixed a week ago. I checked the symbols and the don't use the "_locked" methods so they're pre Bug 1628583 builds.