Commit 9c95c2491cafb85de6ce09c653a5754f0d4cb2f3
2021-11-08T20:01:56
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.
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
diff --git a/src/video/x11/SDL_x11events.c b/src/video/x11/SDL_x11events.c
index 135d22d..57c498d 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 1dae060..835dc15 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)