From 23e4a7060080ebb61f516a2edcd71ab123029c0e Mon Sep 17 00:00:00 2001 From: Derek Foreman Date: Wed, 21 Jul 2021 16:26:00 -0500 Subject: [PATCH] client: Add new proxy marshalling functions with flags There's a race when destroying wayland objects in a multi-threaded client. This occurs because we call: wl_proxy_marshal(foo); wl_proxy_destroy(foo); And each of these functions takes, and releases, the display mutex. Between the two calls, the display is not locked. In order to allow atomically marshalling the proxy and destroying the proxy without releasing the lock, add yet more wl_proxy_marshal_* functions. This time add flags and jam in all existing warts with the hope that we can make it future proof this time. Signed-off-by: Derek Foreman --- src/wayland-client-core.h | 18 ++++++++ src/wayland-client.c | 89 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/wayland-client-core.h b/src/wayland-client-core.h index 0cd96e01..ce91a6f6 100644 --- a/src/wayland-client-core.h +++ b/src/wayland-client-core.h @@ -119,9 +119,27 @@ struct wl_display; */ struct wl_event_queue; +/** Destroy proxy after marshalling + * @ingroup wl_proxy + */ +#define WL_MARSHAL_FLAG_DESTROY (1 << 0) + void wl_event_queue_destroy(struct wl_event_queue *queue); +struct wl_proxy * +wl_proxy_marshal_flags(struct wl_proxy *proxy, uint32_t opcode, + const struct wl_interface *interface, + uint32_t version, + uint32_t flags, ...); + +struct wl_proxy * +wl_proxy_marshal_array_flags(struct wl_proxy *proxy, uint32_t opcode, + const struct wl_interface *interface, + uint32_t version, + uint32_t flags, + union wl_argument *args); + void wl_proxy_marshal(struct wl_proxy *p, uint32_t opcode, ...); diff --git a/src/wayland-client.c b/src/wayland-client.c index d4c8cfa0..63ce0d01 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -737,12 +737,94 @@ wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy, union wl_argument *args, const struct wl_interface *interface, uint32_t version) +{ + return wl_proxy_marshal_array_flags(proxy, opcode, interface, version, 0, args); +} + +/** Prepare a request to be sent to the compositor + * + * \param proxy The proxy object + * \param opcode Opcode of the request to be sent + * \param interface The interface to use for the new proxy + * \param version The protocol object version of the new proxy + * \param flags Flags that modify marshalling behaviour + * \param ... Extra arguments for the given request + * \return A new wl_proxy for the new_id argument or NULL on error + * + * Translates the request given by opcode and the extra arguments into the + * wire format and write it to the connection buffer. + * + * For new-id arguments, this function will allocate a new wl_proxy + * and send the ID to the server. The new wl_proxy will be returned + * on success or NULL on error with errno set accordingly. The newly + * created proxy will have the version specified. + * + * The flag WL_MARSHAL_FLAG_DESTROY may be passed to ensure the proxy + * is destroyed atomically with the marshalling in order to prevent + * races that can occur if the display lock is dropped between the + * marshal and destroy operations. + * + * \note This should not normally be used by non-generated code. + * + * \memberof wl_proxy + */ +WL_EXPORT struct wl_proxy * +wl_proxy_marshal_flags(struct wl_proxy *proxy, uint32_t opcode, + const struct wl_interface *interface, uint32_t version, + uint32_t flags, ...) +{ + union wl_argument args[WL_CLOSURE_MAX_ARGS]; + va_list ap; + + va_start(ap, flags); + wl_argument_from_va_list(proxy->object.interface->methods[opcode].signature, + args, WL_CLOSURE_MAX_ARGS, ap); + va_end(ap); + + return wl_proxy_marshal_array_flags(proxy, opcode, interface, version, flags, args); +} + +/** Prepare a request to be sent to the compositor + * + * \param proxy The proxy object + * \param opcode Opcode of the request to be sent + * \param interface The interface to use for the new proxy + * \param version The protocol object version for the new proxy + * \param flags Flags that modify marshalling behaviour + * \param args Extra arguments for the given request + * + * Translates the request given by opcode and the extra arguments into the + * wire format and write it to the connection buffer. This version takes an + * array of the union type wl_argument. + * + * For new-id arguments, this function will allocate a new wl_proxy + * and send the ID to the server. The new wl_proxy will be returned + * on success or NULL on error with errno set accordingly. The newly + * created proxy will have the version specified. + * + * The flag WL_MARSHAL_FLAG_DESTROY may be passed to ensure the proxy + * is destroyed atomically with the marshalling in order to prevent + * races that can occur if the display lock is dropped between the + * marshal and destroy operations. + * + * \note This is intended to be used by language bindings and not in + * non-generated code. + * + * \sa wl_proxy_marshal_flags() + * + * \memberof wl_proxy + */ +WL_EXPORT struct wl_proxy * +wl_proxy_marshal_array_flags(struct wl_proxy *proxy, uint32_t opcode, + const struct wl_interface *interface, uint32_t version, + uint32_t flags, union wl_argument *args) { struct wl_closure *closure; struct wl_proxy *new_proxy = NULL; const struct wl_message *message; + struct wl_display *disp = proxy->display; - pthread_mutex_lock(&proxy->display->mutex); + pthread_mutex_lock(&disp->mutex); message = &proxy->object.interface->methods[opcode]; if (interface) { @@ -775,7 +857,10 @@ wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy, wl_closure_destroy(closure); err_unlock: - pthread_mutex_unlock(&proxy->display->mutex); + if (flags & WL_MARSHAL_FLAG_DESTROY) + wl_proxy_destroy_caller_locks(proxy); + + pthread_mutex_unlock(&disp->mutex); return new_proxy; }