master 9c62ffb 2/3: Fix lock failures in xg_select

[Lars Ingebrigtsen]

branch: master
commit 9c62ffb08262c82b7e38e6eb5767f2087424aa47
Author: Pip Cet <pipcet@gmail.com>
Commit: Lars Ingebrigtsen <larsi@gnus.org>

    Fix lock failures in xg_select
    
    * src/xgselect.c (release_select_lock, acquire_select_lock):
    Introduce.
    (xg_select): Use `acquire_select_lock', `release_select_lock'.
    * src/thread.c (release_select_lock): Introduce for non-GLib builds.
    (really_call_select): Call `release_select_lock'.  Simplify by
    ensuring acquisition of the lock always succeeds (bug#36609).
---
 src/thread.c   |  8 ++++++++
 src/xgselect.c | 42 ++++++++++++++++++++++++++++--------------
 src/xgselect.h |  2 ++
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/src/thread.c b/src/thread.c
index b638dd7..b4d8a53 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -28,6 +28,12 @@ along with GNU Emacs.  If not, see 
<https://www.gnu.org/licenses/>.  */
 #include "pdumper.h"
 #include "keyboard.h"
 
+#ifdef HAVE_GLIB
+#include <xgselect.h>
+#else
+#define release_select_lock() do { } while (0)
+#endif
+
 union aligned_thread_state
 {
   struct thread_state s;
@@ -586,6 +592,8 @@ really_call_select (void *arg)
   sa->result = (sa->func) (sa->max_fds, sa->rfds, sa->wfds, sa->efds,
                           sa->timeout, sa->sigmask);
 
+  release_select_lock ();
+
   block_interrupt_signal (&oldset);
   /* If we were interrupted by C-g while inside sa->func above, the
      signal handler could have called maybe_reacquire_global_lock, in
diff --git a/src/xgselect.c b/src/xgselect.c
index f8d0bac..be70107 100644
--- a/src/xgselect.c
+++ b/src/xgselect.c
@@ -29,6 +29,27 @@ along with GNU Emacs.  If not, see 
<https://www.gnu.org/licenses/>.  */
 #include "blockinput.h"
 #include "systime.h"
 
+static ptrdiff_t threads_holding_glib_lock;
+static GMainContext *glib_main_context;
+
+void release_select_lock (void)
+{
+  if (--threads_holding_glib_lock == 0)
+    g_main_context_release (glib_main_context);
+}
+
+static void acquire_select_lock (GMainContext *context)
+{
+  if (threads_holding_glib_lock++ == 0)
+    {
+      glib_main_context = context;
+      while (!g_main_context_acquire (context))
+       {
+         /* Spin. */
+       }
+    }
+}
+
 /* `xg_select' is a `pselect' replacement.  Why do we need a separate function?
    1. Timeouts.  Glib and Gtk rely on timer events.  If we did pselect
       with a greater timeout then the one scheduled by Glib, we would
@@ -54,26 +75,19 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set 
*efds,
   GPollFD *gfds = gfds_buf;
   int gfds_size = ARRAYELTS (gfds_buf);
   int n_gfds, retval = 0, our_fds = 0, max_fds = fds_lim - 1;
-  bool context_acquired = false;
   int i, nfds, tmo_in_millisec, must_free = 0;
   bool need_to_dispatch;
 
   context = g_main_context_default ();
-  context_acquired = g_main_context_acquire (context);
-  /* FIXME: If we couldn't acquire the context, we just silently proceed
-     because this function handles more than just glib file descriptors.
-     Note that, as implemented, this failure is completely silent: there is
-     no feedback to the caller.  */
+  acquire_select_lock (context);
 
   if (rfds) all_rfds = *rfds;
   else FD_ZERO (&all_rfds);
   if (wfds) all_wfds = *wfds;
   else FD_ZERO (&all_wfds);
 
-  n_gfds = (context_acquired
-           ? g_main_context_query (context, G_PRIORITY_LOW, &tmo_in_millisec,
-                                   gfds, gfds_size)
-           : -1);
+  n_gfds = g_main_context_query (context, G_PRIORITY_LOW, &tmo_in_millisec,
+                                gfds, gfds_size);
 
   if (gfds_size < n_gfds)
     {
@@ -151,8 +165,10 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set 
*efds,
 #else
   need_to_dispatch = true;
 #endif
-  if (need_to_dispatch && context_acquired)
+  if (need_to_dispatch)
     {
+      acquire_select_lock (context);
+
       int pselect_errno = errno;
       /* Prevent g_main_dispatch recursion, that would occur without
          block_input wrapper, because event handlers call
@@ -162,11 +178,9 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set 
*efds,
         g_main_context_dispatch (context);
       unblock_input ();
       errno = pselect_errno;
+      release_select_lock ();
     }
 
-  if (context_acquired)
-    g_main_context_release (context);
-
   /* To not have to recalculate timeout, return like this.  */
   if ((our_fds > 0 || (nfds == 0 && tmop == &tmo)) && (retval == 0))
     {
diff --git a/src/xgselect.h b/src/xgselect.h
index a38591f..512bf3a 100644
--- a/src/xgselect.h
+++ b/src/xgselect.h
@@ -29,4 +29,6 @@ extern int xg_select (int max_fds,
                      fd_set *rfds, fd_set *wfds, fd_set *efds,
                      struct timespec *timeout, sigset_t *sigmask);
 
+extern void release_select_lock (void);
+
 #endif /* XGSELECT_H */