Fix #1281: glfwPostEmptyEvent sometimes doesn't wake up glfwWaitEvents on X11.

This is a rare race that triggers often when running GLFW programs on GitHub Actions with Xvfb.

The underlying issue is internal to Xlib: XSendEvent races with an external select() call and may get dispatched before select() has blocked on the display fd.

The fix observes a pipe in the select() call, and writes to that pipe from glfwPostEmptyEvent to make select() return.

The original issue reproduces locally once per minute when running the windowjs/windowjs tests in a loop;
The bug hasn't triggered after running for several hours with the fix.
This commit is contained in:
Joao da Silva 2022-01-25 22:43:35 +01:00
parent df8d7bc892
commit 1bef01f210
4 changed files with 91 additions and 6 deletions

View File

@ -121,6 +121,7 @@ information on what to include when reporting a bug.
## Changelog
- Bugfix: fixed glfwPostEmptyEvent not unblocking glfwWaitEvents on X11 (#1281)
- Added `GLFW_PLATFORM` init hint for runtime platform selection (#1958)
- Added `GLFW_ANY_PLATFORM`, `GLFW_PLATFORM_WIN32`, `GLFW_PLATFORM_COCOA`,
`GLFW_PLATFORM_WAYLAND`, `GLFW_PLATFORM_X11` and `GLFW_PLATFORM_NULL` symbols to

View File

@ -35,6 +35,7 @@
#include <stdio.h>
#include <locale.h>
#include <unistd.h>
#include <fcntl.h>
// Translate the X11 KeySyms for a key to a GLFW key code
@ -1053,6 +1054,35 @@ static int errorHandler(Display *display, XErrorEvent* event)
return 0;
}
static inline GLFWbool selfPipe(int fds[2])
{
if (pipe(fds) != 0)
return GLFW_FALSE;
int i;
for (i = 0; i < 2; i++)
{
int flags = fcntl(fds[i], F_GETFD);
if (flags == -1)
break;
if (fcntl(fds[i], F_SETFD, flags | FD_CLOEXEC) == -1)
break;
flags = fcntl(fds[i], F_GETFL);
if (flags == -1)
break;
if (fcntl(fds[i], F_SETFL, flags | O_NONBLOCK) == -1)
break;
}
if (i == 2)
return GLFW_TRUE;
close(fds[0]);
close(fds[1]);
return GLFW_FALSE;
}
//////////////////////////////////////////////////////////////////////////
////// GLFW internal API //////
@ -1509,6 +1539,13 @@ int _glfwInitX11(void)
}
_glfwPollMonitorsX11();
if (!selfPipe(_glfw.x11.eventLoopData.pipe))
{
_glfwInputError(GLFW_PLATFORM_ERROR, "X11: failed to create pipe");
return GLFW_FALSE;
}
return GLFW_TRUE;
}
@ -1594,6 +1631,18 @@ void _glfwTerminateX11(void)
_glfw.x11.xi.handle = NULL;
}
if (_glfw.x11.eventLoopData.pipe[0] > 0)
{
close(_glfw.x11.eventLoopData.pipe[0]);
_glfw.x11.eventLoopData.pipe[0] = -1;
}
if (_glfw.x11.eventLoopData.pipe[1] > 0)
{
close(_glfw.x11.eventLoopData.pipe[1]);
_glfw.x11.eventLoopData.pipe[1] = -1;
}
// NOTE: These need to be unloaded after XCloseDisplay, as they register
// cleanup callbacks that get called by that function
_glfwTerminateEGL();

View File

@ -871,6 +871,10 @@ typedef struct _GLFWlibraryX11
PFN_XShapeQueryVersion QueryVersion;
PFN_XShapeCombineMask ShapeCombineMask;
} xshape;
struct {
int pipe[2];
} eventLoopData;
} _GLFWlibraryX11;
// X11-specific per-monitor data

View File

@ -57,6 +57,16 @@
#define _GLFW_XDND_VERSION 5
static inline void drainPipe(int fd)
{
static char buf[256];
ssize_t len;
do
{
len = read(fd, buf, sizeof(buf));
} while (len > 0 || (len < 0 && errno == EINTR));
}
// Wait for data to arrive using select
// This avoids blocking other threads via the per-display Xlib lock that also
// covers GLX functions
@ -65,16 +75,19 @@ static GLFWbool waitForEvent(double* timeout)
{
fd_set fds;
const int fd = ConnectionNumber(_glfw.x11.display);
int count = fd + 1;
const int pipe = _glfw.x11.eventLoopData.pipe[0];
int max = fd > pipe ? fd : pipe;
#if defined(__linux__)
if (_glfw.linjs.inotify > fd)
count = _glfw.linjs.inotify + 1;
if (_glfw.linjs.inotify > max)
max = _glfw.linjs.inotify;
#endif
for (;;)
{
FD_ZERO(&fds);
FD_SET(fd, &fds);
FD_SET(pipe, &fds);
#if defined(__linux__)
if (_glfw.linjs.inotify > 0)
FD_SET(_glfw.linjs.inotify, &fds);
@ -87,9 +100,12 @@ static GLFWbool waitForEvent(double* timeout)
struct timeval tv = { seconds, microseconds };
const uint64_t base = _glfwPlatformGetTimerValue();
const int result = select(count, &fds, NULL, NULL, &tv);
const int result = select(max + 1, &fds, NULL, NULL, &tv);
const int error = errno;
if (FD_ISSET(pipe, &fds))
drainPipe(pipe);
*timeout -= (_glfwPlatformGetTimerValue() - base) /
(double) _glfwPlatformGetTimerFrequency();
@ -98,8 +114,17 @@ static GLFWbool waitForEvent(double* timeout)
if ((result == -1 && error == EINTR) || *timeout <= 0.0)
return GLFW_FALSE;
}
else if (select(count, &fds, NULL, NULL, NULL) != -1 || errno != EINTR)
return GLFW_TRUE;
else
{
const int result = select(max + 1, &fds, NULL, NULL, NULL);
const int error = errno;
if (FD_ISSET(pipe, &fds))
drainPipe(pipe);
if (result != -1 || error != EINTR)
return GLFW_TRUE;
}
}
}
@ -2805,6 +2830,12 @@ void _glfwPostEmptyEventX11(void)
XSendEvent(_glfw.x11.display, _glfw.x11.helperWindowHandle, False, 0, &event);
XFlush(_glfw.x11.display);
int result;
do
{
result = write(_glfw.x11.eventLoopData.pipe[1], "x", 1);
} while (result < 0 && errno == EINTR);
}
void _glfwGetCursorPosX11(_GLFWwindow* window, double* xpos, double* ypos)