Fix for https://github.com/glfw/glfw/issues/1281

Refactor common code into backend_utils.h

Factorize common logic between wayland and x11, add doc.

Add doc, minor change

Minor change

Change initialization order

Fix wayland compilation

Remove trailing whitespace

src/CMakeLists.txt

@@ -18,13 +18,13 @@ elseif (_GLFW_WIN32)
                      win32_monitor.c win32_time.c win32_thread.c win32_window.c
                      wgl_context.c egl_context.c osmesa_context.c)
 elseif (_GLFW_X11)
-    set(glfw_HEADERS ${common_HEADERS} x11_platform.h xkb_unicode.h posix_time.h
+    set(glfw_HEADERS ${common_HEADERS} x11_platform.h xkb_unicode.h unix_commons.h posix_time.h
                      posix_thread.h glx_context.h egl_context.h osmesa_context.h)
     set(glfw_SOURCES ${common_SOURCES} x11_init.c x11_monitor.c x11_window.c
                      xkb_unicode.c posix_time.c posix_thread.c glx_context.c
                      egl_context.c osmesa_context.c)
 elseif (_GLFW_WAYLAND)
-    set(glfw_HEADERS ${common_HEADERS} wl_platform.h
+    set(glfw_HEADERS ${common_HEADERS} wl_platform.h unix_commons.h
                      posix_time.h posix_thread.h xkb_unicode.h egl_context.h
                      osmesa_context.h)
     set(glfw_SOURCES ${common_SOURCES} wl_init.c wl_monitor.c wl_window.c

src/unix_commons.h

@@ -0,0 +1,102 @@
+//========================================================================
+// GLFW 3.3 Unix commons - www.glfw.org
+//------------------------------------------------------------------------
+// Copyright (c) 2014 Jonas Ådahl <jadahl@gmail.com>
+//
+// This software is provided 'as-is', without any express or implied
+// warranty. In no event will the authors be held liable for any damages
+// arising from the use of this software.
+//
+// Permission is granted to anyone to use this software for any purpose,
+// including commercial applications, and to alter it and redistribute it
+// freely, subject to the following restrictions:
+//
+// 1. The origin of this software must not be misrepresented; you must not
+//    claim that you wrote the original software. If you use this software
+//    in a product, an acknowledgment in the product documentation would
+//    be appreciated but is not required.
+//
+// 2. Altered source versions must be plainly marked as such, and must not
+//    be misrepresented as being the original software.
+//
+// 3. This notice may not be removed or altered from any source
+//    distribution.
+//
+//========================================================================
+
+#pragma once
+#include <fcntl.h>
+#include <unistd.h>
+#include <poll.h>
+#include <errno.h>
+
+#ifdef __NetBSD__
+#define ppoll pollts
+#endif
+
+static inline int
+initWakeup(int fds[2])
+{
+    return pipe2(fds, O_CLOEXEC | O_NONBLOCK);
+}
+
+static inline void
+wakeUp(int fd) {
+    while (write(fd, "w", 1) < 0 && errno == EINTR);
+}
+
+/*
+ * Uses ppoll to wait until an event occurs.
+ *
+ * If an event is received from the "wakeup" file descriptor,
+ *   this file descriptor is drained.
+ *
+ * @param timeout: timeout in seconds. strictly negative
+ *     values mean "no timeout".
+ */
+static inline int
+ppollWithTimeout(struct pollfd *fds, nfds_t nfds, double timeout)
+{
+    for (nfds_t i = 0; i < nfds; i++)
+    {
+        fds[i].revents = 0;
+    }
+
+    int res;
+    if(timeout >= 0.0)
+    {
+        const long seconds = (long) timeout;
+        const long nanoseconds = (long) ((timeout - seconds) * 1e9);
+        struct timespec tv = { seconds, nanoseconds };
+        res = ppoll(fds, nfds, &tv, NULL);
+    }
+    else
+    {
+        res = ppoll(fds, nfds, NULL, NULL);
+    }
+
+    if(res > 0)
+    {
+        if (fds[0].revents & POLLIN)
+        {
+            // an empty event has been posted: now that we are woken up,
+            // we can ignore other potential empty events.
+            static char drain_buf[64];
+            while(read(fds[0].fd, drain_buf, sizeof(drain_buf)) < 0 && errno == EINTR);
+        }
+    }
+    return res;
+}
+
+static inline void
+closeFds(int *fds, size_t count)
+{
+    while(count--) {
+        if (*fds > 0) {
+            close(*fds);
+            *fds = -1;
+        }
+        fds++;
+    }
+}
+

src/wl_init.c

@@ -24,7 +24,9 @@
 //
 //========================================================================
 
+#define _GNU_SOURCE
 #include "internal.h"
+#include "unix_commons.h"
 
 #include <assert.h>
 #include <linux/input.h>
@@ -34,6 +36,7 @@
 #include <sys/mman.h>
 #include <sys/timerfd.h>
 #include <unistd.h>
+#include <fcntl.h>
 #include <wayland-client.h>
 
 
@@ -939,6 +942,13 @@ static void createKeyTables(void)
 
 int _glfwPlatformInit(void)
 {
+    if(initWakeup(_glfw.wl.eventLoopData.wakeupFds) != 0)
+    {
+        _glfwInputError(GLFW_PLATFORM_ERROR,
+                        "Wayland: failed to create self pipe");
+        return GLFW_FALSE;
+    }
+
     _glfw.wl.cursor.handle = _glfw_dlopen("libwayland-cursor.so.0");
     if (!_glfw.wl.cursor.handle)
     {
@@ -1054,9 +1064,30 @@ int _glfwPlatformInit(void)
     _glfwInitTimerPOSIX();
 
     _glfw.wl.timerfd = -1;
+    // The repeat_info event is only available since wl_keyboard version 4,
+    // so there is no need to create a timer if we won’t have any way to use it,
+    // see: https://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml#n2184
     if (_glfw.wl.seatVersion >= 4)
         _glfw.wl.timerfd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
 
+    _glfw.wl.eventLoopData.fds[0].events = POLLIN;
+    _glfw.wl.eventLoopData.fds[0].fd = _glfw.wl.eventLoopData.wakeupFds[0];
+
+    _glfw.wl.eventLoopData.fds[1].events = POLLIN;
+    _glfw.wl.eventLoopData.fds[1].fd = wl_display_get_fd(_glfw.wl.display);
+
+    _glfw.wl.eventLoopData.fds[2].events = POLLIN;
+    if (_glfw.wl.seatVersion >= 4)
+    {
+        _glfw.wl.eventLoopData.nFds = 3;
+        _glfw.wl.eventLoopData.fds[2].fd = _glfw.wl.timerfd;
+    }
+    else
+    {
+      _glfw.wl.eventLoopData.nFds = 2;
+      _glfw.wl.eventLoopData.fds[2].fd = 0;
+    }
+
     if (_glfw.wl.pointer && _glfw.wl.shm)
     {
         _glfw.wl.cursorTheme = wl_cursor_theme_load(NULL, 32, _glfw.wl.shm);
@@ -1142,6 +1173,8 @@ void _glfwPlatformTerminate(void)
         wl_display_flush(_glfw.wl.display);
         wl_display_disconnect(_glfw.wl.display);
     }
+    closeFds(_glfw.wl.eventLoopData.wakeupFds
+           , sizeof(_glfw.wl.eventLoopData.wakeupFds)/sizeof(_glfw.wl.eventLoopData.wakeupFds[0]));
 }
 
 const char* _glfwPlatformGetVersionString(void)

src/wl_platform.h

@@ -30,6 +30,7 @@
 #include <xkbcommon/xkbcommon-compose.h>
 #endif
 #include <dlfcn.h>
+#include <poll.h>
 
 typedef VkFlags VkWaylandSurfaceCreateFlagsKHR;
 
@@ -312,6 +313,12 @@ typedef struct _GLFWlibraryWayland
         PFN_wl_egl_window_resize window_resize;
     } egl;
 
+    struct {
+        nfds_t nFds; // the number of fds that need to be polled
+        struct pollfd fds[3];
+        int wakeupFds[2];
+    } eventLoopData;
+
 } _GLFWlibraryWayland;
 
 // Wayland-specific per-monitor data

src/wl_window.c

@@ -27,16 +27,14 @@
 #define _GNU_SOURCE
 
 #include "internal.h"
+#include "unix_commons.h"
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <errno.h>
-#include <unistd.h>
 #include <string.h>
-#include <fcntl.h>
 #include <sys/mman.h>
 #include <sys/timerfd.h>
-#include <poll.h>
 
 
 static void handlePing(void* data,
@@ -691,15 +689,9 @@ static GLFWbool createXdgSurface(_GLFWwindow* window)
 }
 
 static void
-handleEvents(int timeout)
+handleEvents(double timeout)
 {
     struct wl_display* display = _glfw.wl.display;
-    struct pollfd fds[] = {
-        { wl_display_get_fd(display), POLLIN },
-        { _glfw.wl.timerfd, POLLIN },
-    };
-    ssize_t read_ret;
-    uint64_t repeats, i;
 
     while (wl_display_prepare_read(display) != 0)
         wl_display_dispatch_pending(display);
@@ -719,9 +711,16 @@ handleEvents(int timeout)
         return;
     }
 
-    if (poll(fds, 2, timeout) > 0)
+    if (ppollWithTimeout(_glfw.wl.eventLoopData.fds
+                       , _glfw.wl.eventLoopData.nFds
+                       , timeout) <= 0)
     {
-        if (fds[0].revents & POLLIN)
+        // ppoll errored or timeout-ed
+        wl_display_cancel_read(display);
+        return;
+    }
+
+    if(_glfw.wl.eventLoopData.fds[1].revents & POLLIN)
     {
         wl_display_read_events(display);
         wl_display_dispatch_pending(display);
@@ -731,22 +730,18 @@ handleEvents(int timeout)
         wl_display_cancel_read(display);
     }
 
-        if (fds[1].revents & POLLIN)
+    if (_glfw.wl.seatVersion >= 4 &&
+        (_glfw.wl.eventLoopData.fds[2].revents & POLLIN))
     {
-            read_ret = read(_glfw.wl.timerfd, &repeats, sizeof(repeats));
+        uint64_t repeats;
-            if (read_ret != 8)
+        ssize_t read_ret = read(_glfw.wl.timerfd, &repeats, sizeof(repeats));
+        if (read_ret != sizeof(repeats))
             return;
-
+        for (uint64_t i = 0; i < repeats; ++i)
-            for (i = 0; i < repeats; ++i)
             _glfwInputKey(_glfw.wl.keyboardFocus, _glfw.wl.keyboardLastKey,
                           _glfw.wl.keyboardLastScancode, GLFW_REPEAT,
                           _glfw.wl.xkb.modifiers);
     }
-    }
-    else
-    {
-        wl_display_cancel_read(display);
-    }
 }
 
 // Translates a GLFW standard cursor to a theme cursor name
@@ -1176,22 +1171,26 @@ void _glfwPlatformSetWindowOpacity(_GLFWwindow* window, float opacity)
 
 void _glfwPlatformPollEvents(void)
 {
-    handleEvents(0);
+    handleEvents(0.0);
 }
 
 void _glfwPlatformWaitEvents(void)
 {
-    handleEvents(-1);
+    handleEvents(-1.0);
 }
 
 void _glfwPlatformWaitEventsTimeout(double timeout)
 {
-    handleEvents((int) (timeout * 1e3));
+    handleEvents(timeout);
 }
 
 void _glfwPlatformPostEmptyEvent(void)
 {
     wl_display_sync(_glfw.wl.display);
+    // To avoid a race condition between glfwPostEmptyEvent and glfwWaitEvents[Timeout]
+    // (see https://github.com/glfw/glfw/issues/1281),
+    // we use a secondary mechanism to notify that an empty event has been posted:
+    wakeUp(_glfw.wl.eventLoopData.wakeupFds[1]);
 }
 
 void _glfwPlatformGetCursorPos(_GLFWwindow* window, double* xpos, double* ypos)

src/x11_init.c

@@ -25,7 +25,9 @@
 //
 //========================================================================
 
+#define _GNU_SOURCE
 #include "internal.h"
+#include "unix_commons.h"
 
 #include <X11/Xresource.h>
 
@@ -34,6 +36,7 @@
 #include <limits.h>
 #include <stdio.h>
 #include <locale.h>
+#include <fcntl.h>
 
 
 // Translate an X11 key code to a GLFW key code.
@@ -913,7 +916,6 @@ Cursor _glfwCreateCursorX11(const GLFWimage* image, int xhot, int yhot)
     return cursor;
 }
 
-
 //////////////////////////////////////////////////////////////////////////
 //////                       GLFW platform API                      //////
 //////////////////////////////////////////////////////////////////////////
@@ -951,6 +953,31 @@ int _glfwPlatformInit(void)
         return GLFW_FALSE;
     }
 
+    if(initWakeup(_glfw.x11.eventLoopData.wakeupFds) != 0)
+    {
+      _glfwInputError(GLFW_PLATFORM_ERROR,
+                      "X11: failed to create self pipe");
+      return GLFW_FALSE;
+    }
+
+    _glfw.x11.eventLoopData.nFds = 2;
+
+    _glfw.x11.eventLoopData.fds[0].events = POLLIN;
+    _glfw.x11.eventLoopData.fds[0].fd = _glfw.x11.eventLoopData.wakeupFds[0];
+
+    _glfw.x11.eventLoopData.fds[1].events = POLLIN;
+    _glfw.x11.eventLoopData.fds[1].fd = ConnectionNumber(_glfw.x11.display);
+
+    _glfw.x11.eventLoopData.fds[2].events = POLLIN;
+    _glfw.x11.eventLoopData.fds[2].fd = 0;
+#if defined (__linux__)
+    if(_glfw.linjs.inotify > 0)
+    {
+        _glfw.x11.eventLoopData.nFds = 3;
+        _glfw.x11.eventLoopData.fds[2].fd = _glfw.linjs.inotify;
+    }
+#endif
+
     _glfw.x11.screen = DefaultScreen(_glfw.x11.display);
     _glfw.x11.root = RootWindow(_glfw.x11.display, _glfw.x11.screen);
     _glfw.x11.context = XUniqueContext();
@@ -1018,10 +1045,16 @@ void _glfwPlatformTerminate(void)
         _glfw.x11.im = NULL;
     }
 
+    closeFds(_glfw.x11.eventLoopData.wakeupFds
+             , sizeof(_glfw.x11.eventLoopData.wakeupFds)/
+                sizeof(_glfw.x11.eventLoopData.wakeupFds[0]));
+
+
     if (_glfw.x11.display)
     {
         XCloseDisplay(_glfw.x11.display);
         _glfw.x11.display = NULL;
+        _glfw.x11.eventLoopData.fds[0].fd = -1;
     }
 
     if (_glfw.x11.x11xcb.handle)

src/x11_platform.h

@@ -29,6 +29,7 @@
 #include <signal.h>
 #include <stdint.h>
 #include <dlfcn.h>
+#include <poll.h>
 
 #include <X11/Xlib.h>
 #include <X11/keysym.h>
@@ -399,6 +400,12 @@ typedef struct _GLFWlibraryX11
         PFN_XRenderFindVisualFormat FindVisualFormat;
     } xrender;
 
+    struct {
+        nfds_t nFds; // the number of fds that need to be polled
+        struct pollfd fds[3];
+        int wakeupFds[2];
+    } eventLoopData;
+
 } _GLFWlibraryX11;
 
 // X11-specific per-monitor data

src/x11_window.c

@@ -25,13 +25,13 @@
 //
 //========================================================================
 
+#define _GNU_SOURCE
 #include "internal.h"
+#include "unix_commons.h"
 
 #include <X11/cursorfont.h>
 #include <X11/Xmd.h>
 
-#include <sys/select.h>
-
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -50,50 +50,57 @@
 
 #define _GLFW_XDND_VERSION 5
 
+static int waitForEventOnceWithPpoll(double* pTimeout)
+{
+    if (pTimeout)
+    {
+        const uint64_t base = _glfwPlatformGetTimerValue();
 
-// Wait for data to arrive using select
+        int r = ppollWithTimeout(_glfw.x11.eventLoopData.fds
+                               , _glfw.x11.eventLoopData.nFds
+                               , *pTimeout);
+
+        const uint64_t now = _glfwPlatformGetTimerValue();
+        *pTimeout -= (now - base) / (double) _glfwPlatformGetTimerFrequency();
+        return r;
+    }
+    else
+    {
+        return ppollWithTimeout(_glfw.x11.eventLoopData.fds
+                              , _glfw.x11.eventLoopData.nFds
+                              , -1.0);
+    }
+}
+
+// Wait for data to arrive using ppoll
 // This avoids blocking other threads via the per-display Xlib lock that also
 // covers GLX functions
 //
-static GLFWbool waitForEvent(double* timeout)
+static GLFWbool waitForEvent(double* pTimeout)
 {
-    fd_set fds;
-    const int fd = ConnectionNumber(_glfw.x11.display);
-    int count = fd + 1;
-
-#if defined(__linux__)
-    if (_glfw.linjs.inotify > fd)
-        count = _glfw.linjs.inotify + 1;
-#endif
     for (;;)
     {
-        FD_ZERO(&fds);
+        int res = waitForEventOnceWithPpoll(pTimeout);
-        FD_SET(fd, &fds);
-#if defined(__linux__)
-        if (_glfw.linjs.inotify > 0)
-            FD_SET(_glfw.linjs.inotify, &fds);
-#endif
 
-        if (timeout)
+        if(res > 0)
         {
-            const long seconds = (long) *timeout;
+            // ppoll was successfull
-            const long microseconds = (long) ((*timeout - seconds) * 1e6);
-            struct timeval tv = { seconds, microseconds };
-            const uint64_t base = _glfwPlatformGetTimerValue();
-
-            const int result = select(count, &fds, NULL, NULL, &tv);
-            const int error = errno;
-
-            *timeout -= (_glfwPlatformGetTimerValue() - base) /
-                (double) _glfwPlatformGetTimerFrequency();
-
-            if (result > 0)
             return GLFW_TRUE;
-            if ((result == -1 && error == EINTR) || *timeout <= 0.0)
+        }
+        else if (res == 0)
+        {
+            // ppoll timed out
+            return GLFW_FALSE;
+        }
+        else if (errno != EINTR && errno != EAGAIN)
+        {
+            return GLFW_FALSE;
+        }
+        else if (pTimeout && *pTimeout <= 0.0)
+        {
+            // the timeout expired
             return GLFW_FALSE;
         }
-        else if (select(count, &fds, NULL, NULL, NULL) != -1 || errno != EINTR)
-            return GLFW_TRUE;
     }
 }
 
@@ -2730,6 +2737,11 @@ void _glfwPlatformPostEmptyEvent(void)
 
     XSendEvent(_glfw.x11.display, _glfw.x11.helperWindowHandle, False, 0, &event);
     XFlush(_glfw.x11.display);
+
+    // To avoid a race condition between glfwPostEmptyEvent and glfwWaitEvents[Timeout]
+    // (see https://github.com/glfw/glfw/issues/1281),
+    // we use a secondary mechanism to notify that an empty event has been posted:
+    wakeUp(_glfw.x11.eventLoopData.wakeupFds[1]);
 }
 
 void _glfwPlatformGetCursorPos(_GLFWwindow* window, double* xpos, double* ypos)