GSSubprocess: Correctly close pipefd in parent

Otherwise we leak...

4 files changed, 21 insertions, 14 deletions

diff --git a/gsystem-subprocess-context-private.h b/gsystem-subprocess-context-private.h
index b51ddb4..719df45 100644
--- a/gsystem-subprocess-context-private.h
+++ b/gsystem-subprocess-context-private.h
@@ -53,7 +53,8 @@ struct _GSSubprocessContext
   gint stderr_fd;
   gchar *stderr_path;
 
-  GArray *pipefds;
+  GArray *postfork_close_fds;
+  GArray *inherit_fds;
 
   GSpawnChildSetupFunc child_setup_func;
   gpointer child_setup_data;
diff --git a/gsystem-subprocess-context.c b/gsystem-subprocess-context.c
index 96de25b..cf5eac8 100644
--- a/gsystem-subprocess-context.c
+++ b/gsystem-subprocess-context.c
@@ -160,7 +160,8 @@ gs_subprocess_context_init (GSSubprocessContext  *self)
   self->stderr_fd = -1;
   self->stdout_disposition = GS_SUBPROCESS_STREAM_DISPOSITION_INHERIT;
   self->stderr_disposition = GS_SUBPROCESS_STREAM_DISPOSITION_INHERIT;
-  self->pipefds = g_array_new (FALSE, FALSE, sizeof (int));
+  self->postfork_close_fds = g_array_new (FALSE, FALSE, sizeof (int));
+  self->inherit_fds = g_array_new (FALSE, FALSE, sizeof (int));
 }
 
 static void
@@ -176,7 +177,8 @@ gs_subprocess_context_finalize (GObject *object)
   g_free (self->stdout_path);
   g_free (self->stderr_path);
 
-  g_array_unref (self->pipefds);
+  g_array_unref (self->postfork_close_fds);
+  g_array_unref (self->inherit_fds);
 
   if (G_OBJECT_CLASS (gs_subprocess_context_parent_class)->finalize != NULL)
     G_OBJECT_CLASS (gs_subprocess_context_parent_class)->finalize (object);
@@ -260,14 +262,14 @@ gs_subprocess_context_class_init (GSSubprocessContextClass *class)
  */
 void
 gs_subprocess_context_argv_append (GSSubprocessContext  *self,
-                                   gchar                *arg)
+                                   const gchar          *arg)
 {
   GPtrArray *new_argv = g_ptr_array_new ();
   gchar **iter;
 
   for (iter = self->argv; *iter; iter++)
     g_ptr_array_add (new_argv, *iter);
-  g_ptr_array_add (new_argv, arg);
+  g_ptr_array_add (new_argv, g_strdup (arg));
   g_ptr_array_add (new_argv, NULL);
 
   /* Don't free elements */
@@ -437,7 +439,8 @@ open_pipe_internal (GSSubprocessContext         *self,
       *out_stream = g_unix_output_stream_new (pipefds[1], TRUE);
       *out_fdno = pipefds[0];
     }
-  g_array_append_val (self->pipefds, *out_fdno);
+  g_array_append_val (self->inherit_fds, *out_fdno);
+  g_array_append_val (self->postfork_close_fds, *out_fdno);
 
   return TRUE;
 }
@@ -446,7 +449,7 @@ open_pipe_internal (GSSubprocessContext         *self,
  * gs_subprocess_context_open_pipe_read:
  * @self:
  * @out_stream: (out) (transfer full): A newly referenced output stream
- * @out_fdno: File descriptor number for the subprocess side of the pipe
+ * @out_fdno: (out): File descriptor number for the subprocess side of the pipe
  *
  * This allows you to open a pipe between the parent and child
  * processes, independent of the standard streams.  For this function,
@@ -475,7 +478,7 @@ gs_subprocess_context_open_pipe_read (GSSubprocessContext         *self,
  * gs_subprocess_context_open_pipe_write:
  * @self:
  * @out_stream: (out) (transfer full): A newly referenced stream
- * @out_fdno: File descriptor number for the subprocess side of the pipe
+ * @out_fdno: (out): File descriptor number for the subprocess side of the pipe
  *
  * Like gs_subprocess_context_open_pipe_read(), but returns a writable
  * channel from which the child process can read.
diff --git a/gsystem-subprocess-context.h b/gsystem-subprocess-context.h
index c35d491..eaec315 100644
--- a/gsystem-subprocess-context.h
+++ b/gsystem-subprocess-context.h
@@ -64,7 +64,7 @@ GSSubprocessContext * gs_subprocess_context_new_argv0 (const gchar   *argv0,
 #endif
 
 void             gs_subprocess_context_argv_append (GSSubprocessContext  *self,
-                                                    gchar                *arg);
+                                                    const gchar          *arg);
 
 /* Environment */
 
diff --git a/gsystem-subprocess.c b/gsystem-subprocess.c
index 27c9ad8..8263b6e 100644
--- a/gsystem-subprocess.c
+++ b/gsystem-subprocess.c
@@ -280,7 +280,7 @@ unix_open_file (const char  *filename,
 typedef struct
 {
   gint                   fds[3];
-  GArray                *pipefds;
+  GArray                *inherit_fds;
   GSpawnChildSetupFunc   child_setup_func;
   gpointer               child_setup_data;
 } ChildData;
@@ -309,10 +309,10 @@ child_setup (gpointer user_data)
         }
     }
 
-  /* Unset the CLOEXEC flag on each of our pipes */
-  for (i = 0; i < child_data->pipefds->len; i++)
+  /* Unset the CLOEXEC flag for the child *should* inherit */
+  for (i = 0; i < child_data->inherit_fds->len; i++)
     {
-      int fd = g_array_index (child_data->pipefds, int, i);
+      int fd = g_array_index (child_data->inherit_fds, int, i);
       int flags;
 
       do
@@ -420,7 +420,7 @@ initable_init (GInitable     *initable,
   else
     g_assert_not_reached ();
 
-  child_data.pipefds = self->context->pipefds;
+  child_data.inherit_fds = self->context->inherit_fds;
 
   if (self->context->keep_descriptors)
     spawn_flags |= G_SPAWN_LEAVE_DESCRIPTORS_OPEN;
@@ -458,6 +458,9 @@ out:
     if (close_fds[i] != -1)
       close (close_fds[i]);
 
+  for (i = 0; i < self->context->postfork_close_fds->len; i++)
+    (void) close (g_array_index (self->context->postfork_close_fds, int, i));
+
   self->stdin_pipe = platform_output_stream_from_spawn_fd (pipe_fds[0]);
   self->stdout_pipe = platform_input_stream_from_spawn_fd (pipe_fds[1]);
   self->stderr_pipe = platform_input_stream_from_spawn_fd (pipe_fds[2]);