From 25f9e32b0e923de18435772464a06abb82b1663c Mon Sep 17 00:00:00 2001
From: David Gow <david@ingeniumdigital.com>
Date: Sat, 2 Oct 2021 16:52:43 +0800
Subject: [PATCH] wayland: Don't let multiple threads dispatch wayland events
 at once

wl_display_dispatch() will block if there are no events available, and
while we try to avoid this by using SDL_IOReady() to verify there are
events before calling it, there is a race condition between
SDL_IOReady() and wl_display_dispatch() if multiple threads are
involved.

This is made more likely by the fact that SDL_GL_SwapWindow() calls
wl_display_dispatch() if vsync is enabled, in order to wait for frame
events. Therefore any program which pumps events on a different thread
from SDL_GL_SwapWindow() could end up blocking in one or other of them
until another event arrives.

This change fixes this by wrapping wl_display_dispatch() in a new mutex,
which ensures only one thread can compete for wayland events at a time,
and hence the SDL_IOReady() check should successfully prevent either
from blocking.
---
 src/video/wayland/SDL_waylandevents.c   | 10 ++++++++++
 src/video/wayland/SDL_waylandopengles.c | 11 ++++++++++-
 src/video/wayland/SDL_waylandvideo.c    |  3 +++
 src/video/wayland/SDL_waylandvideo.h    |  1 +
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/video/wayland/SDL_waylandevents.c b/src/video/wayland/SDL_waylandevents.c
index 4cfd0f1cd..799e5fb9b 100644
--- a/src/video/wayland/SDL_waylandevents.c
+++ b/src/video/wayland/SDL_waylandevents.c
@@ -231,11 +231,21 @@ Wayland_PumpEvents(_THIS)
         keyboard_repeat_handle(&input->keyboard_repeat, now);
     }
 
+    /* If we're trying to dispatch the display in another thread,
+     * we could trigger a race condition and end up blocking
+     * in wl_display_dispatch() */
+    if (SDL_TryLockMutex(d->display_dispatch_lock)) {
+        return;
+    }
+
     if (SDL_IOReady(WAYLAND_wl_display_get_fd(d->display), SDL_FALSE, 0)) {
         err = WAYLAND_wl_display_dispatch(d->display);
     } else {
         err = WAYLAND_wl_display_dispatch_pending(d->display);
     }
+
+    SDL_UnlockMutex(d->display_dispatch_lock);
+
     if (err == -1 && !d->display_disconnected) {
         /* Something has failed with the Wayland connection -- for example,
          * the compositor may have shut down and closed its end of the socket,
diff --git a/src/video/wayland/SDL_waylandopengles.c b/src/video/wayland/SDL_waylandopengles.c
index 23429dbbd..a6cb5027f 100644
--- a/src/video/wayland/SDL_waylandopengles.c
+++ b/src/video/wayland/SDL_waylandopengles.c
@@ -127,7 +127,8 @@ Wayland_GLES_SwapWindow(_THIS, SDL_Window *window)
 
     /* Control swap interval ourselves. See comments on Wayland_GLES_SetSwapInterval */
     if (swap_interval != 0) {
-        struct wl_display *display = ((SDL_VideoData *)_this->driverdata)->display;
+        SDL_VideoData *videodata = (SDL_VideoData *)_this->driverdata;
+        struct wl_display *display = videodata->display;
         SDL_VideoDisplay *sdldisplay = SDL_GetDisplayForWindow(window);
         /* ~10 frames (or 1 sec), so we'll progress even if throttled to zero. */
         const Uint32 max_wait = SDL_GetTicks() + (sdldisplay->current_mode.refresh_rate ?
@@ -148,12 +149,20 @@ Wayland_GLES_SwapWindow(_THIS, SDL_Window *window)
                 break;
             }
 
+            /* Make sure we're not competing with SDL_PumpEvents() for any new
+             * events, or one of us may end up blocking in wl_display_dispatch */
+            if (SDL_TryLockMutex(videodata->display_dispatch_lock)) {
+                continue;
+            }
+
             if (SDL_IOReady(WAYLAND_wl_display_get_fd(display), SDL_FALSE, max_wait - now) <= 0) {
                 /* Error or timeout expired without any events for us */
+                SDL_UnlockMutex(videodata->display_dispatch_lock);
                 break;
             }
 
             WAYLAND_wl_display_dispatch(display);
+            SDL_UnlockMutex(videodata->display_dispatch_lock);
         }
         SDL_AtomicSet(&data->swap_interval_ready, 0);
     }
diff --git a/src/video/wayland/SDL_waylandvideo.c b/src/video/wayland/SDL_waylandvideo.c
index 03464d193..df915eea6 100644
--- a/src/video/wayland/SDL_waylandvideo.c
+++ b/src/video/wayland/SDL_waylandvideo.c
@@ -169,6 +169,7 @@ Wayland_DeleteDevice(SDL_VideoDevice *device)
         WAYLAND_wl_display_flush(data->display);
         WAYLAND_wl_display_disconnect(data->display);
     }
+    SDL_DestroyMutex(data->display_dispatch_lock);
     SDL_free(data);
     SDL_free(device);
     SDL_WAYLAND_UnloadSymbols();
@@ -201,6 +202,8 @@ Wayland_CreateDevice(int devindex)
 
     data->display = display;
 
+    data->display_dispatch_lock = SDL_CreateMutex();
+
     /* Initialize all variables that we clean on shutdown */
     device = SDL_calloc(1, sizeof(SDL_VideoDevice));
     if (!device) {
diff --git a/src/video/wayland/SDL_waylandvideo.h b/src/video/wayland/SDL_waylandvideo.h
index 2d8b6323a..42da52e0e 100644
--- a/src/video/wayland/SDL_waylandvideo.h
+++ b/src/video/wayland/SDL_waylandvideo.h
@@ -86,6 +86,7 @@ typedef struct {
     char *classname;
 
     int relative_mouse_mode;
+    SDL_mutex *display_dispatch_lock;
 } SDL_VideoData;
 
 typedef struct {