Closed Bug 1607088 Opened 1 year ago Closed 1 year ago

[Wayland] Frequent crashes under gdk_wayland_window_ref_cairo_surface when spawning the inspector on a different window

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox-esr68 --- unaffected
firefox72 --- unaffected
firefox73 --- fixed
firefox74 --- fixed

People

(Reporter: emilio, Assigned: stransky)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Crash Data

Attachments

(1 file)

Blocks: wayland

This is with WebRender, fwiw.

This is the assert: assert (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&surface->ref_count));, so either we're reffing the surface too late, or too early. May be a gdk bug...

I've been seeing a different cairo assert, not related to the inspector or webrender, just sometimes when launching. Only after recent updates.

(firefox:14792): Gdk-WARNING **: 20:28:20.829: (gdkwindow-wayland.c:810):buffer_release_callback: runtime check failed: (impl->staging_cairo_surface != cairo_surface)
Assertion failed: (status == cr->status), function _cairo_create_in_error, file cairo.c, line 412.

I managed to catch this one under rr, so tentatively taking.

Assignee: nobody → emilio
(rr) p/x surface->ref_count.ref_count
$4 = 0xe5e5e5e5

Yeah that's not good.

This is failing: https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gdk/wayland/gdkwindow-wayland.c#L810

And we're freeing a surface too early, I think.

I think this might be a regression caused by the wl_surface_commit added by bug 1606776, perhaps in combination with the wl_surface_commit added by bug 1542808, which possibly runs on another thread. GTK and Wayland are not thread-safe.

Regressed by: 1606776

I see asserts from the line Emilio pointed out (but with the crash signature from bug 1607033) doing various things, like trying to dock the inspector, opening the page info dialog, or opening an external link.

See Also: → 1607033

(In reply to Jan Alexander Steffens [:heftig] from comment #7)

I think this might be a regression caused by the wl_surface_commit added by bug 1606776, perhaps in combination with the wl_surface_commit added by bug 1542808, which possibly runs on another thread. GTK and Wayland are not thread-safe.

I do think it's because of that bug, but I don't think it's a threading issue. At least I don't see anything relevant going on off the main thread. What I see is that we're hitting this after that wl_surface_commit runs, which confuses gtk.

So I think that commit added by bug 1606776 is just messing up with the gdk state (at least when gdkwindow's state is in pending_buffer_attached state), making wayland send a wl_release_buffer that gdk thinks it is due to its own commit but it's not.

Now what the right thing to do is, I don't know. It may be that we just need to addref the cairo surface so that gdk releases it on their buffer_release_callback, or it may be a deeper problem...

Assignee: emilio → nobody

We need to force Gtk to re-commit GdkWindow wl_surface to compositor. We may use frame begin/end or request frame clock repaint, whatever it takes. I'll look at it.

Assignee: nobody → stransky
Priority: -- → P2
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3aaf0cc32ae9
[Wayland] Request mShell window repaint instead of direct commit to wl_surface, r=heftig
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

Please nominate this for Beta approval if necessary.

Flags: needinfo?(stransky)
Duplicate of this bug: 1607524
Duplicate of this bug: 1607033
Depends on: 1606751
Flags: needinfo?(stransky)

Comment on attachment 9118971 [details]
Bug 1607088 [Wayland] Request mShell window repaint instead of direct commit to wl_surface, r?heftig

Beta/Release Uplift Approval Request

  • User impact if declined: Crashes on Wayland backend caused by commit to wl_surface owned by Gtk.
  • 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: Bug 1606751
  • 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
Attachment #9118971 - Flags: approval-mozilla-beta?
Crash Signature: [@ __GI_raise]

Comment on attachment 9118971 [details]
Bug 1607088 [Wayland] Request mShell window repaint instead of direct commit to wl_surface, r?heftig

Wayland crash fix. Approved for 73.0b3.

Attachment #9118971 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1608117
Blocks: 1589611
You need to log in before you can comment on or make changes to this bug.