From: Alexandros Frantzis alexandros.frantzis@collabora.com
In our setup with a dedicated event dispatch thread, libwayland ensures that object proxies associated with an event handler remain valid (or NULL) while the handler is executing. However, no such guarantees are given for the proxy user data. It is thus possible for the user data to become invalid (e.g., its memory freed from a different thread) right after the event handler is entered.
This is an issue for wayland_surface associated proxies since they may receive unsolicited events from the compositor at any time (e.g., xdg_surface.configure), even while we are destroying the wayland_surface.
To avoid the problem, we add a Wayland event loop synchronization operation after the destruction of wayland_surface related proxies, but before we destroy the wayland_surface itself. The sync operation guarantees that all libwayland event dispatches after it will take into account the destroyed proxies, and thus will not provide access to these proxies or their user data. Event dispatches before the sync will have valid user data since the wayland_surface is still not destroyed at that point. --- dlls/winewayland.drv/wayland.c | 152 +++++++++++++++++++++++++ dlls/winewayland.drv/wayland_surface.c | 4 + dlls/winewayland.drv/waylanddrv.h | 7 ++ dlls/winewayland.drv/waylanddrv_main.c | 4 +- 4 files changed, 164 insertions(+), 3 deletions(-)
diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 357dc4208d3..5d2fe122368 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -28,7 +28,11 @@
#include "wine/debug.h"
+#include <errno.h> +#include <fcntl.h> +#include <poll.h> #include <stdlib.h> +#include <unistd.h>
WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv);
@@ -38,6 +42,8 @@ struct wayland process_wayland = .output_mutex = PTHREAD_MUTEX_INITIALIZER };
+static int wayland_callback_pipe[2]; + /********************************************************************** * xdg_wm_base handling */ @@ -135,6 +141,16 @@ BOOL wayland_process_init(void)
TRACE("wl_display=%p\n", process_wayland.wl_display);
+ /* Make the read end of the callback pipe non-blocking. */ + if (pipe(wayland_callback_pipe) == -1 || + fcntl(wayland_callback_pipe[0], F_SETFD, FD_CLOEXEC) == -1 || + fcntl(wayland_callback_pipe[0], F_SETFL, O_NONBLOCK) == -1 || + fcntl(wayland_callback_pipe[1], F_SETFD, FD_CLOEXEC) == -1) + { + ERR("Failed to create callback pipe fds\n"); + return FALSE; + } + if (!(process_wayland.wl_event_queue = wl_display_create_queue(process_wayland.wl_display))) { ERR("Failed to create event queue\n"); @@ -183,3 +199,139 @@ BOOL wayland_process_init(void)
return TRUE; } + +struct wayland_callback +{ + pthread_mutex_t mutex; + pthread_cond_t done_cond; + BOOL done; +}; + +static int wayland_dispatch_callbacks(void) +{ + struct wayland_callback *cb; + int nread; + + while (TRUE) + { + do + { + nread = read(wayland_callback_pipe[0], &cb, sizeof(cb)); + } while (nread == -1 && errno == EINTR); + + if (nread == -1 && errno == EAGAIN) break; + + if (nread < sizeof(cb)) + { + TRACE("Failed to read callback nread=%d errno=%d\n", nread, errno); + return -1; + } + + pthread_mutex_lock(&cb->mutex); + cb->done = TRUE; + pthread_cond_signal(&cb->done_cond); + pthread_mutex_unlock(&cb->mutex); + } + + return 0; +} + +/********************************************************************** + * wayland_dispatch_events + * + * Dispatch the events received from the compositor and any scheduled + * internal callbacks. + */ +int wayland_dispatch_events(void) +{ + struct pollfd pfd[2] = {0}; + int ret; + struct wl_display *display = process_wayland.wl_display; + struct wl_event_queue *queue = process_wayland.wl_event_queue; + + pfd[0].fd = wl_display_get_fd(display); + + if (wl_display_prepare_read_queue(display, queue) == -1) + return wl_display_dispatch_queue_pending(display, queue); + + while (TRUE) + { + ret = wl_display_flush(display); + + if (ret != -1 || errno != EAGAIN) break; + + pfd[0].events = POLLOUT; + while ((ret = poll(pfd, 1, -1)) == -1 && errno == EINTR) continue; + + if (ret == -1) + { + wl_display_cancel_read(display); + return -1; + } + } + + if (ret == -1 && errno != EPIPE) + { + wl_display_cancel_read(display); + return -1; + } + + /* Events from the compositor */ + pfd[0].events = POLLIN; + pfd[0].revents = 0; + /* Internal callbacks */ + pfd[1].events = POLLIN; + pfd[1].fd = wayland_callback_pipe[0]; + + while ((ret = poll(pfd, 2, -1)) == -1 && errno == EINTR) continue; + + if (!(pfd[0].revents & POLLIN)) wl_display_cancel_read(display); + + if (ret == -1) return -1; + + /* Handle wl_display fd input. */ + if (pfd[0].revents & POLLIN) + { + if (wl_display_read_events(display) == -1) return -1; + if (wl_display_dispatch_queue_pending(display, queue) == -1) return -1; + } + + /* Handle callbacks. */ + if (pfd[1].revents & POLLIN) + if (wayland_dispatch_callbacks() == -1) return -1; + + return 0; +} + +/********************************************************************** + * wayland_sync + * + * Wait for all currently queued Wayland events to be handled. + */ +void wayland_sync(void) +{ + struct wayland_callback cb = + { + .mutex = PTHREAD_MUTEX_INITIALIZER, + .done_cond = PTHREAD_COND_INITIALIZER, + .done = FALSE + }; + struct wayland_callback *cb_ptr = &cb; + int nwritten; + + do + { + nwritten = write(wayland_callback_pipe[1], &cb_ptr, sizeof(cb_ptr)); + } while (nwritten == -1 && errno == EINTR); + + if (nwritten < sizeof(cb_ptr)) + { + ERR("Failed to write callback data to pipe nwritten=%d errno=%d\n", + nwritten, errno); + return; + } + + pthread_mutex_lock(&cb.mutex); + while (!cb.done) pthread_cond_wait(&cb.done_cond, &cb.mutex); + pthread_mutex_unlock(&cb.mutex); +} diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 82e6ce869e6..b23a1694e4a 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -124,6 +124,10 @@ void wayland_surface_destroy(struct wayland_surface *surface)
wl_display_flush(process_wayland.wl_display);
+ /* Sync to ensure no event handlers are running, or are going to run, for + * the just destroyed objects. */ + wayland_sync(); + pthread_cond_destroy(&surface->configured_cond); pthread_mutex_destroy(&surface->mutex);
diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 4c42dcd6268..ba084a1f3c4 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -112,6 +112,13 @@ struct wayland_surface BOOL wayland_process_init(void) DECLSPEC_HIDDEN; void wayland_init_display_devices(BOOL force) DECLSPEC_HIDDEN;
+/********************************************************************** + * Wayland event loop + */ + +int wayland_dispatch_events(void) DECLSPEC_HIDDEN; +void wayland_sync(void) DECLSPEC_HIDDEN; + /********************************************************************** * Wayland output */ diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index 530cdd83d90..3154e2d5752 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -56,9 +56,7 @@ err:
static NTSTATUS waylanddrv_unix_read_events(void *arg) { - while (wl_display_dispatch_queue(process_wayland.wl_display, - process_wayland.wl_event_queue) != -1) - continue; + while (wayland_dispatch_events() != -1) continue; /* This function only returns on a fatal error, e.g., if our connection * to the Wayland server is lost. */ return STATUS_UNSUCCESSFUL;