client: Lock display when setting a proxy event queue
authorAlexandros Frantzis <alexandros.frantzis@collabora.com>
Thu, 26 May 2022 09:18:28 +0000 (12:18 +0300)
committerSimon Ser <contact@emersion.fr>
Thu, 9 Jun 2022 18:38:53 +0000 (18:38 +0000)
Assignments to a wl_proxy's queue member are currently not synchronized
with potential reads of that member during event reading/queuing.
Assuming atomic pointer value reads and writes (which is a reasonable
assumption), and using the documented best practices to handle event
queue changes, a queue change should still be safe to perform.

That being said, such implicitly atomic accesses are difficult to assess
for correctness, especially since they do not introduce memory barriers.

To make the code more obviously correct, and handle any potential races
we are not currently aware of, this commit updates wl_proxy_set_queue()
to set the proxy's event queue under the display lock (all other
proxy queue accesses are already done under the display lock).

Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
src/wayland-client.c

index 3e42f70..90fb9c7 100644 (file)
@@ -2333,12 +2333,16 @@ wl_proxy_get_class(struct wl_proxy *proxy)
 WL_EXPORT void
 wl_proxy_set_queue(struct wl_proxy *proxy, struct wl_event_queue *queue)
 {
+       pthread_mutex_lock(&proxy->display->mutex);
+
        if (queue) {
                assert(proxy->display == queue->display);
                proxy->queue = queue;
        } else {
                proxy->queue = &proxy->display->default_queue;
        }
+
+       pthread_mutex_unlock(&proxy->display->mutex);
 }
 
 /** Create a proxy wrapper for making queue assignments thread-safe