From 1e987cb92ea646282a2e3f7c085f96ae7eeea425 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Camilla=20L=C3=B6wy?= Date: Fri, 18 Feb 2022 15:19:16 +0100 Subject: [PATCH] X11: Fix joystick events causing busy waiting On Linux, the inotify descriptor was included in the set used for select, but could not break the outer loop, leading to busy waiting until timeout or the correct X11 event arrived. This commit adds a new function for waiting just on X11 events. Fixes #1872 --- README.md | 1 + src/x11_window.c | 59 +++++++++++++++++++++++++++++------------------- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 7fc32b98..23cc069a 100644 --- a/README.md +++ b/README.md @@ -270,6 +270,7 @@ information on what to include when reporting a bug. - [X11] Bugfix: Dynamic loading on OpenBSD failed due to soname differences - [X11] Bugfix: Waiting for events would fail if file descriptor was too large (#2024) + - [X11] Bugfix: Joystick events could lead to busy-waiting (#1872) - [Wayland] Added dynamic loading of all Wayland libraries - [Wayland] Added support for key names via xkbcommon - [Wayland] Removed support for `wl_shell` (#1443) diff --git a/src/x11_window.c b/src/x11_window.c index f4adbd3e..122db2ba 100644 --- a/src/x11_window.c +++ b/src/x11_window.c @@ -56,26 +56,12 @@ #define _GLFW_XDND_VERSION 5 - -// Wait for event data to arrive on any relevant file descriptor -// This avoids blocking other threads via the per-display Xlib lock that also -// covers GLX functions +// Wait for data to arrive on any of the specified file descriptors // -static GLFWbool waitForEvent(double* timeout) +static GLFWbool waitForData(struct pollfd* fds, nfds_t count, double* timeout) { for (;;) { - nfds_t count = 1; - struct pollfd fds[2] = - { - { ConnectionNumber(_glfw.x11.display), POLLIN } - }; - -#if defined(__linux__) - if (_glfw.joysticksInitialized) - fds[count++] = (struct pollfd) { _glfw.linjs.inotify, POLLIN }; -#endif - if (timeout) { const int milliseconds = (int) (*timeout * 1e3); @@ -105,6 +91,33 @@ static GLFWbool waitForEvent(double* timeout) } } +// Wait for event data to arrive on the X11 display socket +// This avoids blocking other threads via the per-display Xlib lock that also +// covers GLX functions +// +static GLFWbool waitForX11Event(double* timeout) +{ + struct pollfd fd = { ConnectionNumber(_glfw.x11.display), POLLIN }; + return waitForData(&fd, 1, timeout); +} + +// Wait for event data to arrive on any event file descriptor +// This avoids blocking other threads via the per-display Xlib lock that also +// covers GLX functions +// +static GLFWbool waitForAnyEvent(double* timeout) +{ + nfds_t count = 1; + struct pollfd fds[2] = { { ConnectionNumber(_glfw.x11.display), POLLIN } }; + +#if defined(__linux__) + if (_glfw.joysticksInitialized) + fds[count++] = (struct pollfd) { _glfw.linjs.inotify, POLLIN }; +#endif + + return waitForData(fds, count, timeout); +} + // Waits until a VisibilityNotify event arrives for the specified window or the // timeout period elapses (ICCCM section 4.2.2) // @@ -118,7 +131,7 @@ static GLFWbool waitForVisibilityNotify(_GLFWwindow* window) VisibilityNotify, &dummy)) { - if (!waitForEvent(&timeout)) + if (!waitForX11Event(&timeout)) return GLFW_FALSE; } @@ -960,7 +973,7 @@ static const char* getSelectionString(Atom selection) SelectionNotify, ¬ification)) { - waitForEvent(NULL); + waitForX11Event(NULL); } if (notification.xselection.property == None) @@ -996,7 +1009,7 @@ static const char* getSelectionString(Atom selection) isSelPropNewValueNotify, (XPointer) ¬ification)) { - waitForEvent(NULL); + waitForX11Event(NULL); } XFree(data); @@ -1898,7 +1911,7 @@ void _glfwPushSelectionToManagerX11(void) } } - waitForEvent(NULL); + waitForX11Event(NULL); } } @@ -2240,7 +2253,7 @@ void _glfwGetWindowFrameSizeX11(_GLFWwindow* window, isFrameExtentsEvent, (XPointer) window)) { - if (!waitForEvent(&timeout)) + if (!waitForX11Event(&timeout)) { _glfwInputError(GLFW_PLATFORM_ERROR, "X11: The window manager has a broken _NET_REQUEST_FRAME_EXTENTS implementation; please report this issue"); @@ -2782,7 +2795,7 @@ void _glfwPollEventsX11(void) void _glfwWaitEventsX11(void) { while (!XPending(_glfw.x11.display)) - waitForEvent(NULL); + waitForAnyEvent(NULL); _glfwPollEventsX11(); } @@ -2791,7 +2804,7 @@ void _glfwWaitEventsTimeoutX11(double timeout) { while (!XPending(_glfw.x11.display)) { - if (!waitForEvent(&timeout)) + if (!waitForAnyEvent(&timeout)) break; }