Commit 25f9e32b0e923de18435772464a06abb82b1663c
2021-10-02T16:52:43
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94
diff --git a/src/video/wayland/SDL_waylandevents.c b/src/video/wayland/SDL_waylandevents.c
index 4cfd0f1..799e5fb 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 23429db..a6cb502 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 03464d1..df915ee 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 2d8b632..42da52e 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 {