[glib/wip/lantw/gspawn-optimize-fd-closing-on-aix-and-bsds] gspawn: Optimize fd closing on AIX and BSDs

[Ting-Wei Lan <lantw src gnome org>]

commit d96e1fca37340133f92b899be5e2042ca3e8feea
Author: Sebastian Schwarz <seschwar gmail com>
Date:   Sun Sep 15 12:53:38 2019 +0800

    gspawn: Optimize fd closing on AIX and BSDs
    
    Instead of calling close or fcntl on all possible file descriptors,
    which is slow on systems with very high limit or even no limit on open
    file descriptors, we can use closefrom or fcntl with F_CLOSEM to close
    all unwanted file descriptors with a single system call.
    
    This change only improves the performance when GSpawnChildSetupFunc is
    NULL because there are applications known to abuse GSpawnChildSetupFunc
    to unset FD_CLOEXEC on file descriptors. Since the change mentioned
    above requires closing file descriptors directly, it cannot be used when
    the caller may want to keep some of them open.
    
    This patch was written by Sebastian Schwarz <seschwar gmail com> and
    uploaded to https://gitlab.gnome.org/GNOME/glib/merge_requests/574.
    It was later modified by Ting-Wei Lan <lantw src gnome org> to address
    code review issues.
    
    Fixes: https://gitlab.gnome.org/GNOME/glib/issues/1638

 glib/gspawn.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 74 insertions(+), 17 deletions(-)
---
diff --git a/glib/gspawn.c b/glib/gspawn.c
index 7dd4bdcbb..b8ceb742c 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -47,7 +47,7 @@
 #include <sys/resource.h>
 #endif /* HAVE_SYS_RESOURCE_H */
 
-#ifdef __linux__
+#if defined(__linux__) || defined(__DragonFly__)
 #include <sys/syscall.h>  /* for syscall and SYS_getdents64 */
 #endif
 
@@ -1126,6 +1126,28 @@ set_cloexec (void *data, gint fd)
   return 0;
 }
 
+static gint
+sane_close (gint fd)
+{
+  gint ret;
+
+ retry:
+  ret = close (fd);
+  if (ret < 0 && errno == EINTR)
+    goto retry;
+
+  return ret;
+}
+
+G_GNUC_UNUSED static int
+close_func (void *data, int fd)
+{
+  if (fd >= GPOINTER_TO_INT (data))
+    (void) sane_close (fd);
+
+  return 0;
+}
+
 #ifndef HAVE_FDWALK
 #ifdef __linux__
 struct linux_dirent64
@@ -1200,7 +1222,7 @@ fdwalk (int (*cb)(void *data, int fd), void *data)
             }
         }
 
-      close (dir_fd);
+      sane_close (dir_fd);
       return res;
     }
 
@@ -1223,7 +1245,39 @@ fdwalk (int (*cb)(void *data, int fd), void *data)
 
   return res;
 }
+#endif /* HAVE_FDWALK */
+
+static void
+safe_closefrom (int lowfd)
+{
+#if defined(__FreeBSD__) || defined(__OpenBSD__)
+  /* Use closefrom function provided by the system if it is known to be
+   * async-signal safe.
+   *
+   * FreeBSD: closefrom is included in the list of async-signal safe functions
+   * found in https://man.freebsd.org/sigaction(2).
+   *
+   * OpenBSD: closefrom is not included in the list, but a direct system call
+   * should be safe to use.
+   */
+  (void) closefrom (lowfd);
+#elif defined(__DragonFly__)
+  /* It is unclear whether closefrom function included in DragonFlyBSD libc_r
+   * is safe to use because it calls a lot of library functions. It is also
+   * unclear whether libc_r itself is still being used. Therefore, we do a
+   * direct system call here ourselves to avoid possible issues.
+   */
+  (void) syscall (SYS_closefrom, lowfd);
+#elif defined(F_CLOSEM)
+  /* NetBSD and AIX have a special fcntl command which does the same thing as
+   * closefrom. NetBSD also includes closefrom function, which seems to be a
+   * simple wrapper of the fcntl command.
+   */
+  (void) fcntl (lowfd, F_CLOSEM);
+#else
+  (void) fdwalk (close_func, GINT_TO_POINTER (lowfd));
 #endif
+}
 
 static gint
 sane_dup2 (gint fd1, gint fd2)
@@ -1280,21 +1334,6 @@ do_exec (gint                  child_err_report_fd,
   if (working_directory && chdir (working_directory) < 0)
     write_err_and_exit (child_err_report_fd,
                         CHILD_CHDIR_FAILED);
-
-  /* Close all file descriptors but stdin stdout and stderr as
-   * soon as we exec. Note that this includes
-   * child_err_report_fd, which keeps the parent from blocking
-   * forever on the other end of that pipe.
-   */
-  if (close_descriptors)
-    {
-      fdwalk (set_cloexec, GINT_TO_POINTER(3));
-    }
-  else
-    {
-      /* We need to do child_err_report_fd anyway */
-      set_cloexec (GINT_TO_POINTER(0), child_err_report_fd);
-    }
   
   /* Redirect pipes as required */
   
@@ -1351,6 +1390,24 @@ do_exec (gint                  child_err_report_fd,
       sane_dup2 (write_null, 2);
       close_and_invalidate (&write_null);
     }
+
+  /* Close all file descriptors but stdin, stdout and stderr
+   * before we exec. Note that this includes
+   * child_err_report_fd, which keeps the parent from blocking
+   * forever on the other end of that pipe.
+   */
+  if (close_descriptors)
+    {
+      if (child_setup == NULL)
+        safe_closefrom (3);
+      else
+        fdwalk (set_cloexec, GINT_TO_POINTER (3));
+    }
+  else
+    {
+      /* We need to do child_err_report_fd anyway */
+      set_cloexec (GINT_TO_POINTER (0), child_err_report_fd);
+    }
   
   /* Call user function just before we exec */
   if (child_setup)