Merge branch 'w32-spawn-fds' into 'main'

Implement fd passing for Windows spawn

See merge request GNOME/glib!2458

glib/gspawn-win32-helper.c

@@ -153,6 +153,18 @@ protect_wargv (gint       argc,
   return argc;
 }
 
+static int
+checked_dup2 (int oldfd, int newfd, int report_fd)
+{
+  if (oldfd == newfd)
+    return newfd;
+
+  if (dup2 (oldfd, newfd) == -1)
+    write_err_and_exit (report_fd, CHILD_DUP_FAILED);
+
+  return newfd;
+}
+
 #if (defined (_MSC_VER) && _MSC_VER >= 1400)
 /*
  * This is the (empty) invalid parameter handler
@@ -188,12 +200,14 @@ int
 main (int ignored_argc, char **ignored_argv)
 #endif
 {
+  GHashTable *fds;  /* (element-type int int) */
   int child_err_report_fd = -1;
   int helper_sync_fd = -1;
   int saved_stderr_fd = -1;
   int i;
   int fd;
   int mode;
+  int maxfd = 2;
   gintptr handle;
   int saved_errno;
   gintptr no_error = CHILD_NO_ERROR;
@@ -229,6 +243,7 @@ main (int ignored_argc, char **ignored_argv)
    * which write error messages.
    */
   child_err_report_fd = atoi (argv[ARG_CHILD_ERR_REPORT]);
+  maxfd = MAX (child_err_report_fd, maxfd);
 
   /* Hack to implement G_SPAWN_FILE_AND_ARGV_ZERO. If
    * argv[ARG_CHILD_ERR_REPORT] is suffixed with a '#' it means we get
@@ -244,6 +259,7 @@ main (int ignored_argc, char **ignored_argv)
    * from another process works only if that other process exists.
    */
   helper_sync_fd = atoi (argv[ARG_HELPER_SYNC]);
+  maxfd = MAX (helper_sync_fd, maxfd);
 
   /* argv[ARG_STDIN..ARG_STDERR] are the file descriptor numbers that
    * should be dup2'd to 0, 1 and 2. '-' if the corresponding fd
@@ -255,20 +271,12 @@ main (int ignored_argc, char **ignored_argv)
   else if (argv[ARG_STDIN][0] == 'z')
     {
       fd = open ("NUL:", O_RDONLY);
-      if (fd != 0)
+      checked_dup2 (fd, 0, child_err_report_fd);
-	{
-	  dup2 (fd, 0);
-	  close (fd);
-	}
     }
   else
     {
       fd = atoi (argv[ARG_STDIN]);
-      if (fd != 0)
+      checked_dup2 (fd, 0, child_err_report_fd);
-	{
-	  dup2 (fd, 0);
-	  close (fd);
-	}
     }
 
   if (argv[ARG_STDOUT][0] == '-')
@@ -276,42 +284,30 @@ main (int ignored_argc, char **ignored_argv)
   else if (argv[ARG_STDOUT][0] == 'z')
     {
       fd = open ("NUL:", O_WRONLY);
-      if (fd != 1)
+      checked_dup2 (fd, 1, child_err_report_fd);
-	{
-	  dup2 (fd, 1);
-	  close (fd);
-	}
     }
   else
     {
       fd = atoi (argv[ARG_STDOUT]);
-      if (fd != 1)
+      checked_dup2 (fd, 1, child_err_report_fd);
-	{
-	  dup2 (fd, 1);
-	  close (fd);
-	}
     }
 
   saved_stderr_fd = reopen_noninherited (dup (2), _O_WRONLY);
+  if (saved_stderr_fd == -1)
+    write_err_and_exit (child_err_report_fd, CHILD_DUP_FAILED);
+
+  maxfd = MAX (saved_stderr_fd, maxfd);
   if (argv[ARG_STDERR][0] == '-')
     ; /* Nothing */
   else if (argv[ARG_STDERR][0] == 'z')
     {
       fd = open ("NUL:", O_WRONLY);
-      if (fd != 2)
+      checked_dup2 (fd, 2, child_err_report_fd);
-	{
-	  dup2 (fd, 2);
-	  close (fd);
-	}
     }
   else
     {
       fd = atoi (argv[ARG_STDERR]);
-      if (fd != 2)
+      checked_dup2 (fd, 2, child_err_report_fd);
-	{
-	  dup2 (fd, 2);
-	  close (fd);
-	}
     }
 
   /* argv[ARG_WORKING_DIRECTORY] is the directory in which to run the
@@ -323,12 +319,76 @@ main (int ignored_argc, char **ignored_argv)
   else if (_wchdir (wargv[ARG_WORKING_DIRECTORY]) < 0)
     write_err_and_exit (child_err_report_fd, CHILD_CHDIR_FAILED);
 
+  fds = g_hash_table_new (NULL, NULL);
+  if (argv[ARG_FDS][0] != '-')
+    {
+      gchar **fdsv = g_strsplit (argv[ARG_FDS], ",", -1);
+      gsize i;
+
+      for (i = 0; fdsv[i]; i++)
+        {
+          char *endptr = NULL;
+          int sourcefd, targetfd;
+          gint64 val;
+
+          val = g_ascii_strtoll (fdsv[i], &endptr, 10);
+          g_assert (val <= G_MAXINT32);
+          sourcefd = val;
+          g_assert (endptr != fdsv[i]);
+          g_assert (*endptr == ':');
+          val = g_ascii_strtoll (endptr + 1, &endptr, 10);
+          targetfd = val;
+          g_assert (val <= G_MAXINT32);
+          g_assert (*endptr == '\0');
+
+          maxfd = MAX (maxfd, sourcefd);
+          maxfd = MAX (maxfd, targetfd);
+
+          g_hash_table_insert (fds, GINT_TO_POINTER (targetfd), GINT_TO_POINTER (sourcefd));
+        }
+
+      g_strfreev (fdsv);
+    }
+
+  maxfd++;
+  child_err_report_fd = checked_dup2 (child_err_report_fd, maxfd, child_err_report_fd);
+  maxfd++;
+  helper_sync_fd = checked_dup2 (helper_sync_fd, maxfd, child_err_report_fd);
+  maxfd++;
+  saved_stderr_fd = checked_dup2 (saved_stderr_fd, maxfd, child_err_report_fd);
+
+  {
+    GHashTableIter iter;
+    gpointer sourcefd, targetfd;
+
+    g_hash_table_iter_init (&iter, fds);
+    while (g_hash_table_iter_next (&iter, &targetfd, &sourcefd))
+      {
+        /* If we're doing remapping fd assignments, we need to handle
+         * the case where the user has specified e.g. 5 -> 4, 4 -> 6.
+         * We do this by duping all source fds, taking care to ensure the new
+         * fds are larger than any target fd to avoid introducing new conflicts.
+         */
+        maxfd++;
+        checked_dup2 (GPOINTER_TO_INT (sourcefd), maxfd, child_err_report_fd);
+        g_hash_table_iter_replace (&iter, GINT_TO_POINTER (maxfd));
+      }
+
+    g_hash_table_iter_init (&iter, fds);
+    while (g_hash_table_iter_next (&iter, &targetfd, &sourcefd))
+      checked_dup2 (GPOINTER_TO_INT (sourcefd), GPOINTER_TO_INT (targetfd), child_err_report_fd);
+  }
+
+  g_hash_table_add (fds, GINT_TO_POINTER (child_err_report_fd));
+  g_hash_table_add (fds, GINT_TO_POINTER (helper_sync_fd));
+  g_hash_table_add (fds, GINT_TO_POINTER (saved_stderr_fd));
+
   /* argv[ARG_CLOSE_DESCRIPTORS] is "y" if file descriptors from 3
    *  upwards should be closed
    */
   if (argv[ARG_CLOSE_DESCRIPTORS][0] == 'y')
     for (i = 3; i < 1000; i++)	/* FIXME real limit? */
-      if (i != child_err_report_fd && i != helper_sync_fd && i != saved_stderr_fd)
+      if (!g_hash_table_contains (fds, GINT_TO_POINTER (i)))
         if (_get_osfhandle (i) != -1)
           close (i);
 
@@ -337,6 +397,8 @@ main (int ignored_argc, char **ignored_argv)
    */
   child_err_report_fd = reopen_noninherited (child_err_report_fd, _O_WRONLY);
   helper_sync_fd = reopen_noninherited (helper_sync_fd, _O_RDONLY);
+  if (helper_sync_fd == -1)
+    write_err_and_exit (child_err_report_fd, CHILD_DUP_FAILED);
 
   /* argv[ARG_WAIT] is "w" to wait for the program to exit */
   if (argv[ARG_WAIT][0] == 'w')
@@ -384,6 +446,7 @@ main (int ignored_argc, char **ignored_argv)
 
   LocalFree (wargv);
   g_strfreev (argv);
+  g_hash_table_unref (fds);
 
   return 0;
 }

glib/gspawn-win32.c

@@ -89,6 +89,7 @@ enum
   CHILD_CHDIR_FAILED,
   CHILD_SPAWN_FAILED,
   CHILD_SPAWN_NOENT,
+  CHILD_DUP_FAILED,
 };
 
 enum {
@@ -101,6 +102,7 @@ enum {
   ARG_CLOSE_DESCRIPTORS,
   ARG_USE_PATH,
   ARG_WAIT,
+  ARG_FDS,
   ARG_PROGRAM,
   ARG_COUNT = ARG_PROGRAM
 };
@@ -393,6 +395,11 @@ set_child_error (gintptr      report[2],
                    _("Failed to execute child process (%s)"),
                    g_strerror (report[1]));
       break;
+    case CHILD_DUP_FAILED:
+      g_set_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED,
+                   _("Failed to dup() in child process (%s)"),
+                   g_strerror (report[1]));
+      break;
     default:
       g_assert_not_reached ();
     }
@@ -569,6 +576,9 @@ fork_exec (gint                  *exit_status,
            gint                   stdin_fd,
            gint                   stdout_fd,
            gint                   stderr_fd,
+           const gint            *source_fds,
+           const gint            *target_fds,
+           gsize                  n_fds,
            gint                  *err_report,
            GError               **error)
 {
@@ -629,7 +639,8 @@ fork_exec (gint                  *exit_status,
       !(flags & G_SPAWN_STDOUT_TO_DEV_NULL) &&
       !(flags & G_SPAWN_STDERR_TO_DEV_NULL) &&
       (working_directory == NULL || !*working_directory) &&
-      (flags & G_SPAWN_LEAVE_DESCRIPTORS_OPEN))
+      (flags & G_SPAWN_LEAVE_DESCRIPTORS_OPEN) &&
+      n_fds == 0)
     {
       /* We can do without the helper process */
       gboolean retval =
@@ -748,6 +759,21 @@ fork_exec (gint                  *exit_status,
   else
     new_argv[ARG_WAIT] = "w";
 
+  if (n_fds == 0)
+    new_argv[ARG_FDS] = g_strdup ("-");
+  else
+    {
+      GString *fds = g_string_new ("");
+      gsize n;
+
+      for (n = 0; n < n_fds; n++)
+        g_string_append_printf (fds, "%d:%d,", source_fds[n], target_fds[n]);
+
+      /* remove the trailing , */
+      g_string_truncate (fds, fds->len - 1);
+      new_argv[ARG_FDS] = g_string_free (fds, FALSE);
+    }
+
   for (i = 0; i <= argc; i++)
     new_argv[ARG_PROGRAM + i] = protected_argv[i];
 
@@ -774,6 +800,7 @@ fork_exec (gint                  *exit_status,
       g_strfreev (protected_argv);
       g_free (new_argv[0]);
       g_free (new_argv[ARG_WORKING_DIRECTORY]);
+      g_free (new_argv[ARG_FDS]);
       g_free (new_argv);
       g_free (helper_process);
 
@@ -789,6 +816,7 @@ fork_exec (gint                  *exit_status,
       g_strfreev (protected_argv);
       g_free (new_argv[0]);
       g_free (new_argv[ARG_WORKING_DIRECTORY]);
+      g_free (new_argv[ARG_FDS]);
       g_free (new_argv);
       g_free (helper_process);
       g_strfreev ((gchar **) wargv);
@@ -820,6 +848,7 @@ fork_exec (gint                  *exit_status,
 
   g_free (new_argv[0]);
   g_free (new_argv[ARG_WORKING_DIRECTORY]);
+  g_free (new_argv[ARG_FDS]);
   g_free (new_argv);
 
   /* Check if gspawn-win32-helper couldn't be run */
@@ -991,6 +1020,7 @@ g_spawn_sync (const gchar          *working_directory,
                   -1,
                   -1,
                   -1,
+                  NULL, NULL, 0,
                   &reportpipe,
                   error))
     return FALSE;
@@ -1215,6 +1245,7 @@ g_spawn_async_with_pipes (const gchar          *working_directory,
                     -1,
                     -1,
                     -1,
+                    NULL, NULL, 0,
                     NULL,
                     error);
 }
@@ -1256,6 +1287,7 @@ g_spawn_async_with_fds (const gchar          *working_directory,
                     stdin_fd,
                     stdout_fd,
                     stderr_fd,
+                    NULL, NULL, 0,
                     NULL,
                     error);
 
@@ -1293,14 +1325,6 @@ g_spawn_async_with_pipes_and_fds (const gchar           *working_directory,
   g_return_val_if_fail (stdout_pipe_out == NULL || stdout_fd < 0, FALSE);
   g_return_val_if_fail (stderr_pipe_out == NULL || stderr_fd < 0, FALSE);
 
-  /* source_fds/target_fds isn’t supported on Windows at the moment. */
-  if (n_fds != 0)
-    {
-      g_set_error_literal (error, G_SPAWN_ERROR, G_SPAWN_ERROR_INVAL,
-                           "FD redirection is not supported on Windows at the moment");
-      return FALSE;
-    }
-
   return fork_exec (NULL,
                     (flags & G_SPAWN_DO_NOT_REAP_CHILD),
                     working_directory,
@@ -1316,6 +1340,9 @@ g_spawn_async_with_pipes_and_fds (const gchar           *working_directory,
                     stdin_fd,
                     stdout_fd,
                     stderr_fd,
+                    source_fds,
+                    target_fds,
+                    n_fds,
                     NULL,
                     error);
 }

glib/gspawn.c

@@ -772,6 +772,8 @@ g_spawn_async_with_pipes (const gchar          *working_directory,
  * any target FDs which equal @stdin_fd, @stdout_fd or @stderr_fd will overwrite
  * them in the spawned process.
  *
+ * @source_fds is supported on Windows since 2.72.
+ *
  * %G_SPAWN_FILE_AND_ARGV_ZERO means that the first element of @argv is
  * the file to execute, while the remaining elements are the actual
  * argument vector to pass to the file. Normally g_spawn_async_with_pipes()

glib/tests/spawn-singlethread.c

@@ -27,10 +27,10 @@
 #include <locale.h>
 #include <string.h>
 #include <fcntl.h>
+#include <glib/gstdio.h>
 
 #ifdef G_OS_UNIX
 #include <glib-unix.h>
-#include <glib/gstdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -433,12 +433,11 @@ test_spawn_nonexistent (void)
 static void
 test_spawn_fd_assignment_clash (void)
 {
-#if defined(G_OS_UNIX) && defined(F_DUPFD_CLOEXEC)
   int tmp_fd;
   guint i;
-  const guint n_fds = 10;
+#define N_FDS 10
-  gint source_fds[n_fds];
+  gint source_fds[N_FDS];
-  gint target_fds[n_fds];
+  gint target_fds[N_FDS];
   const gchar *argv[] = { "/nonexistent", NULL };
   gboolean retval;
   GError *local_error = NULL;
@@ -449,27 +448,32 @@ test_spawn_fd_assignment_clash (void)
   tmp_fd = g_file_open_tmp ("glib-spawn-test-XXXXXX", NULL, NULL);
   g_assert_cmpint (tmp_fd, >=, 0);
 
-  for (i = 0; i < (n_fds - 1); ++i)
+  for (i = 0; i < (N_FDS - 1); ++i)
     {
-      int source = fcntl (tmp_fd, F_DUPFD_CLOEXEC, 3);
+      int source;
+#ifdef F_DUPFD_CLOEXEC
+      source = fcntl (tmp_fd, F_DUPFD_CLOEXEC, 3);
+#else
+      source = dup (tmp_fd);
+#endif
       g_assert_cmpint (source, >=, 0);
       source_fds[i] = source;
-      target_fds[i] = source + n_fds;
+      target_fds[i] = source + N_FDS;
     }
 
   source_fds[i] = tmp_fd;
-  target_fds[i] = tmp_fd + n_fds;
+  target_fds[i] = tmp_fd + N_FDS;
 
   /* Print out the FD map. */
   g_test_message ("FD map:");
-  for (i = 0; i < n_fds; i++)
+  for (i = 0; i < N_FDS; i++)
     g_test_message (" • %d → %d", source_fds[i], target_fds[i]);
 
   /* Spawn the subprocess. This should fail because the executable doesn’t
    * exist. */
   retval = g_spawn_async_with_pipes_and_fds (NULL, argv, NULL, G_SPAWN_DEFAULT,
                                              NULL, NULL, -1, -1, -1,
-                                             source_fds, target_fds, n_fds,
+                                             source_fds, target_fds, N_FDS,
                                              NULL, NULL, NULL, NULL,
                                              &local_error);
   g_assert_error (local_error, G_SPAWN_ERROR, G_SPAWN_ERROR_NOENT);
@@ -484,11 +488,8 @@ test_spawn_fd_assignment_clash (void)
   g_assert_cmpuint (statbuf.st_size, ==, 0);
 
   /* Clean up. */
-  for (i = 0; i < n_fds; i++)
+  for (i = 0; i < N_FDS; i++)
     g_close (source_fds[i], NULL);
-#else  /* !G_OS_UNIX */
-  g_test_skip ("FD redirection only supported on Unix with F_DUPFD_CLOEXEC");
-#endif  /* !G_OS_UNIX */
 }
 
 int