[glib: 1/2] GSubprocessLauncher: Move cleanup to dispose()

[Philip Withnall <pwithnall src gnome org>]

commit 605cff61da1a7a77b675137c68bbcbb80db46821
Author: Sergio Costas <raster rastersoft com>
Date:   Wed Sep 30 22:35:00 2020 +0200

    GSubprocessLauncher: Move cleanup to dispose()
    
    The GSubprocessLauncher class lacks a dispose() method, and frees
    all their resources in the finalize() method.
    
    This is a problem with Javascript because the sockets passed to a
    child process using g_subprocess_launcher_take_fd() aren't closed
    in the parent space until the object is fully freed. This means
    that if the child closes a socket, it won't be detected until the
    GSubprocessLauncher object has been freed by the garbage
    collector.
    
    Just closing the socket externally is not a valid solution,
    because the finalize() method will close it again, and since
    another file/pipe/socket could have been opened in the meantime
    and use the same FD number, the finalize() method would close
    an incorrect FD.
    
    An example is launching a child process that uses its own
    socket for Wayland: the parent creates two sockets with
    socketpair(), passes one to the Wayland API (wl_client_create()),
    and the other is passed to the child process using
    g_subprocess_launcher_take_fd(). But now there are two instances
    of that second socket: one in the parent, and another one in the
    child process. That means that, if the child closes its socket (or
    dies), the Wayland server will not detect that until the
    GSubprocessLauncher object is fully destroyed. That means that a
    GSubprocessLauncher created in Javascript will last for several
    seconds after the child dies, and every window or graphical element
    will remain in the screen until the Garbage Collector destroys the
    GSubprocessLauncher object.
    
    This patch fixes this by moving the resource free code into a
    dispose() method, which can be called from Javascript. This allows
    to ensure that any socket passed to the child with
    g_subprocess_launcher_take_fd() can be closed even from Javascript
    just by calling the method run_dispose().
    
    Fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1670

 gio/gsubprocesslauncher.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)
---
diff --git a/gio/gsubprocesslauncher.c b/gio/gsubprocesslauncher.c
index 8c2f1c179..4f9bbaa9a 100644
--- a/gio/gsubprocesslauncher.c
+++ b/gio/gsubprocesslauncher.c
@@ -131,47 +131,52 @@ g_subprocess_launcher_set_property (GObject *object, guint prop_id,
 }
 
 static void
-g_subprocess_launcher_finalize (GObject *object)
+g_subprocess_launcher_dispose (GObject *object)
 {
   GSubprocessLauncher *self = G_SUBPROCESS_LAUNCHER (object);
 
 #ifdef G_OS_UNIX
   guint i;
 
-  g_free (self->stdin_path);
-  g_free (self->stdout_path);
-  g_free (self->stderr_path);
+  g_clear_pointer (&self->stdin_path, g_free);
+  g_clear_pointer (&self->stdout_path, g_free);
+  g_clear_pointer (&self->stderr_path, g_free);
 
   if (self->stdin_fd != -1)
     close (self->stdin_fd);
+  self->stdin_fd = -1;
 
   if (self->stdout_fd != -1)
     close (self->stdout_fd);
+  self->stdout_fd = -1;
 
   if (self->stderr_fd != -1)
     close (self->stderr_fd);
+  self->stderr_fd = -1;
 
   if (self->basic_fd_assignments)
     {
       for (i = 0; i < self->basic_fd_assignments->len; i++)
         (void) close (g_array_index (self->basic_fd_assignments, int, i));
-      g_array_unref (self->basic_fd_assignments);
+      g_clear_pointer (&self->basic_fd_assignments, g_array_unref);
     }
   if (self->needdup_fd_assignments)
     {
       for (i = 0; i < self->needdup_fd_assignments->len; i += 2)
         (void) close (g_array_index (self->needdup_fd_assignments, int, i));
-      g_array_unref (self->needdup_fd_assignments);
+      g_clear_pointer (&self->needdup_fd_assignments, g_array_unref);
     }
 
   if (self->child_setup_destroy_notify)
     (* self->child_setup_destroy_notify) (self->child_setup_user_data);
+  self->child_setup_destroy_notify = NULL;
+  self->child_setup_user_data = NULL;
 #endif
 
-  g_strfreev (self->envp);
-  g_free (self->cwd);
+  g_clear_pointer (&self->envp, g_strfreev);
+  g_clear_pointer (&self->cwd, g_free);
 
-  G_OBJECT_CLASS (g_subprocess_launcher_parent_class)->finalize (object);
+  G_OBJECT_CLASS (g_subprocess_launcher_parent_class)->dispose (object);
 }
 
 static void
@@ -194,7 +199,7 @@ g_subprocess_launcher_class_init (GSubprocessLauncherClass *class)
   GObjectClass *gobject_class = G_OBJECT_CLASS (class);
 
   gobject_class->set_property = g_subprocess_launcher_set_property;
-  gobject_class->finalize = g_subprocess_launcher_finalize;
+  gobject_class->dispose = g_subprocess_launcher_dispose;
 
   g_object_class_install_property (gobject_class, 1,
                                    g_param_spec_flags ("flags", "Flags", "GSubprocessFlags for launched 
processes",
@@ -634,12 +639,16 @@ g_subprocess_launcher_take_fd (GSubprocessLauncher   *self,
 {
   if (source_fd == target_fd)
     {
-      g_array_append_val (self->basic_fd_assignments, source_fd);
+      if (self->basic_fd_assignments)
+        g_array_append_val (self->basic_fd_assignments, source_fd);
     }
   else
     {
-      g_array_append_val (self->needdup_fd_assignments, source_fd);
-      g_array_append_val (self->needdup_fd_assignments, target_fd);
+      if (self->needdup_fd_assignments)
+        {
+          g_array_append_val (self->needdup_fd_assignments, source_fd);
+          g_array_append_val (self->needdup_fd_assignments, target_fd);
+        }
     }
 }