[glib/th/gspawn-no-safe-close: 10/10] gspawn: avoid race due to retry with EINTR on close()

[Thomas Haller <thaller src gnome org>]

commit da6774f96e9b0e4974bc18eb6eac6339d497f8ba
Author: Thomas Haller <thaller redhat com>
Date:   Wed Oct 12 23:30:37 2022 +0200

    gspawn: avoid race due to retry with EINTR on close()
    
    Retry on EINTR is wrong on many OS, including Linux. See the comment
    in g_close() why that is.
    
    As we cannot use g_close() after fork, we had safe_close(). This had the
    wrong retry loop on EINTR. Drop that.
    
    This was especially problematic since commit 6f46294227f8 ('gspawn: Don’t
    use g_close() in async-signal-safe context'). Before, safe_close() was
    only called after fork, where there is only one thread and there is no
    concern about a race.
    
    Leave safe_close() in place. It is mostly pointless now, as it does
    nothing except calling close(). What it however does, and why it
    is useful to keep, is:
    
    - don't return any result, indicating to the caller they are not
      supposed to handle any error.
    
    - comment on and act as a reminder on the intricacies of close(), (not)
      handling errors, EINTR, g_close() and async-signal-safety.
    
    Fixes: 6f46294227f8 ('gspawn: Don’t use g_close() in async-signal-safe context')

 glib/gspawn.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
---
diff --git a/glib/gspawn.c b/glib/gspawn.c
index 4e029eedf5..c644cebb11 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -162,7 +162,7 @@ extern char **environ;
  */
 
 
-static gint safe_close (gint fd);
+static void safe_close (gint fd);
 
 static gint g_execute (const gchar  *file,
                        gchar       **argv,
@@ -1340,16 +1340,21 @@ dupfd_cloexec (int old_fd, int new_fd_min)
 
 /* This function is called between fork() and exec() and hence must be
  * async-signal-safe (see signal-safety(7)). */
-static gint
+static void
 safe_close (gint fd)
 {
-  gint ret;
-
-  do
-    ret = close (fd);
-  while (ret < 0 && errno == EINTR);
-
-  return ret;
+  /* Note that this function is (also) called after fork(), so it cannot use
+   * g_close().
+   * Note that it is however called both from the parent and the child
+   * process.
+   *
+   * This function returns no error, because there is nothing what the caller
+   * could do with that information. That is even the case for EINTR. See
+   * g_close() about the specialty of EINTR and why that is correct.
+   * If g_close() ever gets extended to handle EINTR specially, then this place
+   * and all other direct calls to close() need updating.
+   */
+  close (fd);
 }
 
 /* This function is called between fork() and exec() and hence must be
@@ -1358,7 +1363,7 @@ G_GNUC_UNUSED static int
 close_func (void *data, int fd)
 {
   if (fd >= GPOINTER_TO_INT (data))
-    (void) safe_close (fd);
+    safe_close (fd);
 
   return 0;
 }