From 9c95c2491cafb85de6ce09c653a5754f0d4cb2f3 Mon Sep 17 00:00:00 2001
From: Cameron Gutman <[EMAIL REDACTED]>
Date: Mon, 8 Nov 2021 20:01:56 -0600
Subject: [PATCH] x11: Use XCheckIfEvent() instead of XNextEvent() for
thread-safety
A racing reader could read from our fd between SDL_IOReady()/X11_Pending()
and our call to XNextEvent() which will cause XNextEvent() to block for
more data. Avoid this by using XCheckIfEvent() which will never block.
This also fixes a bug where we could poll() for data, even when events were
already read and pending in the queue. Unlike the Wayland implementation,
this isn't totally thread-safe because nothing prevents a racing reader
from reading events into the queue between our XCheckIfEvent() and
SDL_IOReady() calls, but I think this is the best we can do with Xlib.
---
src/video/x11/SDL_x11events.c | 45 ++++++++++++++++++-----------------
src/video/x11/SDL_x11sym.h | 1 -
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/src/video/x11/SDL_x11events.c b/src/video/x11/SDL_x11events.c
index 135d22dd36..57c498d3fc 100644
--- a/src/video/x11/SDL_x11events.c
+++ b/src/video/x11/SDL_x11events.c
@@ -1557,23 +1557,21 @@ X11_HandleFocusChanges(_THIS)
}
}
}
-/* Ack! X11_XPending() actually performs a blocking read if no events available */
-static int
-X11_Pending(Display * display)
+
+static Bool
+isAnyEvent(Display *display, XEvent *ev, XPointer arg)
{
- /* Flush the display connection and look to see if events are queued */
- X11_XFlush(display);
- if (X11_XEventsQueued(display, QueuedAlready)) {
- return (1);
- }
+ return True;
+}
- /* More drastic measures are required -- see if X is ready to talk */
- if (SDL_IOReady(ConnectionNumber(display), SDL_IOR_READ, 0)) {
- return (X11_XPending(display));
+static SDL_bool
+X11_PollEvent(Display *display, XEvent *event)
+{
+ if (!X11_XCheckIfEvent(display, event, isAnyEvent, NULL)) {
+ return SDL_FALSE;
}
- /* Oh well, nothing is ready .. */
- return (0);
+ return SDL_TRUE;
}
void
@@ -1611,17 +1609,21 @@ X11_WaitEventTimeout(_THIS, int timeout)
SDL_zero(xevent);
- if (timeout == 0) {
- if (X11_Pending(display)) {
- X11_XNextEvent(display, &xevent);
- } else {
- return 0;
- }
+ /* Flush and poll to grab any events already read and queued */
+ X11_XFlush(display);
+ if (X11_PollEvent(display, &xevent)) {
+ /* Fall through */
+ } else if (timeout == 0) {
+ return 0;
} else {
/* Use SDL_IOR_NO_RETRY to ensure SIGINT will break us out of our wait */
int err = SDL_IOReady(ConnectionNumber(display), SDL_IOR_READ | SDL_IOR_NO_RETRY, timeout);
if (err > 0) {
- X11_XNextEvent(display, &xevent);
+ if (!X11_PollEvent(display, &xevent)) {
+ /* Someone may have beat us to reading the fd. Return 1 here to
+ * trigger the normal spurious wakeup logic in the event core. */
+ return 1;
+ }
} else if (err == 0) {
/* Timeout */
return 0;
@@ -1679,8 +1681,7 @@ X11_PumpEvents(_THIS)
SDL_zero(xevent);
/* Keep processing pending events */
- while (X11_Pending(data->display)) {
- X11_XNextEvent(data->display, &xevent);
+ while (X11_PollEvent(data->display, &xevent)) {
X11_DispatchEvent(_this, &xevent);
}
diff --git a/src/video/x11/SDL_x11sym.h b/src/video/x11/SDL_x11sym.h
index 1dae060e71..835dc15033 100644
--- a/src/video/x11/SDL_x11sym.h
+++ b/src/video/x11/SDL_x11sym.h
@@ -97,7 +97,6 @@ SDL_X11_SYM(int,XMapRaised,(Display* a,Window b),(a,b),return)
SDL_X11_SYM(Status,XMatchVisualInfo,(Display* a,int b,int c,int d,XVisualInfo* e),(a,b,c,d,e),return)
SDL_X11_SYM(int,XMissingExtension,(Display* a,_Xconst char* b),(a,b),return)
SDL_X11_SYM(int,XMoveWindow,(Display* a,Window b,int c,int d),(a,b,c,d),return)
-SDL_X11_SYM(int,XNextEvent,(Display* a,XEvent* b),(a,b),return)
SDL_X11_SYM(Display*,XOpenDisplay,(_Xconst char* a),(a),return)
SDL_X11_SYM(Status,XInitThreads,(void),(),return)
SDL_X11_SYM(int,XPeekEvent,(Display* a,XEvent* b),(a,b),return)