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 788558 - dmabuf crashes old clients
dmabuf crashes old clients
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
git master
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
: 788472 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-10-05 13:49 UTC by Daniel Stone
Modified: 2017-10-05 17:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wayland-dma-buf: Don't send modifiers to old clients (1.30 KB, patch)
2017-10-05 13:51 UTC, Daniel Stone
none Details | Review
wayland-dma-buf: Don't send modifiers to old clients (1.34 KB, patch)
2017-10-05 14:06 UTC, Daniel Stone
none Details | Review
wayland-dma-buf: Don't send modifiers to old clients (1.61 KB, patch)
2017-10-05 14:43 UTC, Daniel Stone
committed Details | Review

Description Daniel Stone 2017-10-05 13:49:29 UTC
Regardless of the zwp_linux_dmabuf_v1 version bound by the client, Mutter will send the 'modifier' event on startup, which is only supported from v3 onwards.

Put a version check around this, so as not to crash clients only supporting the old dmabuf version, including GStreamer's waylandsink.
Comment 1 Daniel Stone 2017-10-05 13:51:29 UTC
Created attachment 360961 [details] [review]
wayland-dma-buf: Don't send modifiers to old clients

The modifier event was only added in v3 of the client; sending it to
older clients (e.g. GStreamer waylandsink) causes them to disconnect
immediately.

Check the client version first before sending any events, replacing it
with the format event for older clients.
Comment 2 Daniel Stone 2017-10-05 14:06:06 UTC
Created attachment 360963 [details] [review]
wayland-dma-buf: Don't send modifiers to old clients

The modifier event was only added in v3 of the client; sending it to
older clients (e.g. GStreamer waylandsink) causes them to disconnect
immediately.

Check the client version first before sending any events, replacing it
with the format event for older clients.
Comment 3 Daniel Stone 2017-10-05 14:06:48 UTC
v2: Emmanuel rightly pointed out I could be using a SINCE_VERSION macro rather than magic numbers.
Comment 4 Jonas Ådahl 2017-10-05 14:17:48 UTC
Review of attachment 360963 [details] [review]:

::: src/wayland/meta-wayland-dma-buf.c
@@ +474,3 @@
   int i;
 
+  zwp_linux_dmabuf_v1_send_format (resource, format);

The commit message said you "replaced" the behaviour for older clients by sending this, but what seems to be happening is that you send it always no matter the version. Which way should it be? As far as I can see, the modifiers event intends to replace the format event, which the commit message also says, but this is not what is happening.

@@ +477,3 @@
+
+  /* The modifier event was only added in v3; v1 and v2 only have the format
+   * event. */

Doesn't the line below just say this? :P

@@ +479,3 @@
+   * event. */
+  if (wl_resource_get_version (resource) < ZWP_LINUX_DMABUF_V1_MODIFIER_SINCE_VERSION)
+     return;

white space issue!
Comment 5 Daniel Stone 2017-10-05 14:43:42 UTC
(In reply to Jonas Ådahl from comment #4)
> ::: src/wayland/meta-wayland-dma-buf.c
> @@ +474,3 @@
>    int i;
>  
> +  zwp_linux_dmabuf_v1_send_format (resource, format);
> 
> The commit message said you "replaced" the behaviour for older clients by
> sending this, but what seems to be happening is that you send it always no
> matter the version. Which way should it be? As far as I can see, the
> modifiers event intends to replace the format event, which the commit
> message also says, but this is not what is happening.

Weston's doesn't send any events to v1 or below, which seems ... suboptimal. I'll send a separate fix for Weston to match this behaviour: send format events to all clients, and send modifier events to v3 or above. The protocol text says that the format event is 'likely' to be deprecated, but nothing about it being totally inert.

> @@ +479,3 @@
> +   * event. */
> +  if (wl_resource_get_version (resource) <
> ZWP_LINUX_DMABUF_V1_MODIFIER_SINCE_VERSION)
> +     return;
> 
> white space issue!

Sorry, going between Mesa's 3 spaces, Mutter's 2/4 spaces, Weston/kernel's 8 space tabs, and the X server's 4 spaces, is melting my brain. :\

I noticed another three-space failure immediately below that, which I'll just squash into v3 of this patch if that's OK.
Comment 6 Daniel Stone 2017-10-05 14:43:49 UTC
Created attachment 360967 [details] [review]
wayland-dma-buf: Don't send modifiers to old clients

The modifier event was only added in v3 of the client; sending it to
older clients (e.g. GStreamer waylandsink) causes them to disconnect
immediately.

Send the older 'format' event to all clients, and only send the newer
'modifier' event to resource versions 3 or above.
Comment 7 Jonas Ådahl 2017-10-05 14:50:12 UTC
Review of attachment 360967 [details] [review]:

lgtm
Comment 8 Jonas Ådahl 2017-10-05 15:00:01 UTC
Attachment 360967 [details] pushed as 32917f1 - wayland-dma-buf: Don't send modifiers to old clients
Comment 9 Florian Müllner 2017-10-05 15:27:02 UTC
*** Bug 788472 has been marked as a duplicate of this bug. ***
Comment 10 Jonas Ådahl 2017-10-05 17:13:59 UTC
Backported to the gnome-3-26 branch as well.