After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 770952 - wayland: Don't destroy cursor backing storage when the buffer is released
wayland: Don't destroy cursor backing storage when the buffer is released
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-09-06 14:07 UTC by Kalev Lember
Modified: 2016-09-08 11:22 UTC
See Also:
GNOME target: 3.22
GNOME version: ---


Attachments
wayland: Don't destroy cursor backing storage when the buffer is released (2.71 KB, patch)
2016-09-06 14:08 UTC, Kalev Lember
reviewed Details | Review

Description Kalev Lember 2016-09-06 14:07:40 UTC
.
Comment 1 Kalev Lember 2016-09-06 14:08:32 UTC
Created attachment 334911 [details] [review]
wayland: Don't destroy cursor backing storage when the buffer is released

As per wayland documentation, a client is free to re-use a buffer and its
backing storage after it is released by the compositor. This commit changes the
wayland cursor cairo_surface destroy logic so that the surface is destroyed
together with the cursor object and kept around meanwhile.

This fixes a cairo_surface refcount underrun and a virt-manager crash reported
in https://bugzilla.redhat.com/show_bug.cgi?id=1373372
Comment 2 Jonas Ådahl 2016-09-07 14:10:44 UTC
Review of attachment 334911 [details] [review]:

I don't think this is the correct solution. Before this patch you could set the cursor and get rid of your cairo_surface_t reference without any unexpected side effects, since the wl_surface implicitly had a reference to the cairo surface via the buffer release callback. With this applied, would we destroy the cairo surface would the wl_buffer also be destroyed (see gdk_wayland_cairo_surface_destroy()).

You write that the issue is a refcount underrun, did you investigate how the ref count went wrong?

For reference, each time one wl_surface.attach(buffer); wl_surface.commit(), we should receive one wl_buffer.release on the attached buffer. Do we cairo_surface_reference() for each time we attach and commit?
Comment 3 Kalev Lember 2016-09-08 11:22:21 UTC
After further debugging it turns out that it was mutter's fault for sending too many release events. Jonas fixed that in https://git.gnome.org/browse/mutter/commit/?id=c7976e0dbcd266d788309aabe51392b7de094b32