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.
This commit is contained in:
Cameron Gutman 2021-11-08 20:01:56 -06:00 committed by Sam Lantinga
parent e9dae813d9
commit 9c95c2491c
2 changed files with 24 additions and 24 deletions

View File

@ -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;
}
static SDL_bool
X11_PollEvent(Display *display, XEvent *event)
{
if (!X11_XCheckIfEvent(display, event, isAnyEvent, NULL)) {
return SDL_FALSE;
}
/* 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));
}
/* 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 {
/* 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);
}

View File

@ -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)