[vte] spawn: Clarify ownership transfer

[Christian Persch <chpe src gnome org>]

commit af9ce0acff15a7714adb8abde9c990d9af4d0856
Author: Christian Persch <chpe src gnome org>
Date:   Thu Oct 22 20:34:52 2020 +0200

    spawn: Clarify ownership transfer
    
    Pass unqiue_ptr<SpawnOperation> to ::run_async to make it clear that it
    takes ownership of the operation.

 src/spawn.cc  | 34 ++++++++++++++++++----------------
 src/spawn.hh  | 16 ++++++++++------
 src/vtepty.cc | 37 +++++++++++++++++++------------------
 3 files changed, 47 insertions(+), 40 deletions(-)
---
diff --git a/src/spawn.cc b/src/spawn.cc
index 6797d470..23d554d9 100644
--- a/src/spawn.cc
+++ b/src/spawn.cc
@@ -724,30 +724,31 @@ SpawnOperation::run_in_thread(GTask* task) noexcept
                 g_task_return_error(task, error.release());
 }
 
-/* Note: this function takes ownership of @this */
 void
-SpawnOperation::run_async(void* source_tag,
+SpawnOperation::run_async(std::unique_ptr<SpawnOperation> op,
+                          void* source_tag,
                           GAsyncReadyCallback callback,
                           void* user_data)
 {
+        /* Spawning is split into the fork() phase, and waiting for the child to
+         * exec or report an error. This is done so that the fork is happening on
+         * the main thread; see issue vte#118.
+         */
+        auto error = vte::glib::Error{};
+        auto rv = op->prepare(error);
+
         /* Create a GTask to run the user-provided callback, and transfers
-         * ownership of @this to the task, meaning that @this will be deleted after
-         * the task is completed.
+         * ownership of @op to the task.
          */
-        auto task = vte::glib::take_ref(g_task_new(context().pty_wrapper(),
-                                                   m_cancellable.get(),
+        auto task = vte::glib::take_ref(g_task_new(op->context().pty_wrapper(),
+                                                   op->m_cancellable.get(),
                                                    callback,
                                                    user_data));
         g_task_set_source_tag(task.get(), source_tag);
-        g_task_set_task_data(task.get(), this, delete_cb);
+        g_task_set_task_data(task.get(), op.release(), delete_cb);
         // g_task_set_name(task.get(), "vte-spawn-async");
 
-        /* Spawning is split into the fork() phase, and waiting for the child to
-         * exec or report an error. This is done so that the fork is happening on
-         * the main thread; see issue vte#118.
-         */
-        auto error = vte::glib::Error{};
-        if (!prepare(error))
+        if (!rv)
                 return g_task_return_error(task.get(), error.release());
 
         /* Async read from the child */
@@ -755,12 +756,13 @@ SpawnOperation::run_async(void* source_tag,
 }
 
 bool
-SpawnOperation::run_sync(GPid* pid,
+SpawnOperation::run_sync(SpawnOperation& op,
+                         GPid* pid,
                          vte::glib::Error& error)
 {
-        auto rv = prepare(error) && run(error);
+        auto rv = op.prepare(error) && op.run(error);
         if (rv)
-                *pid = release_pid();
+                *pid = op.release_pid();
         else
                 *pid = -1;
 
diff --git a/src/spawn.hh b/src/spawn.hh
index 5623050f..6a1b4e58 100644
--- a/src/spawn.hh
+++ b/src/spawn.hh
@@ -210,7 +210,9 @@ private:
 
         static void delete_cb(void* that)
         {
-                delete reinterpret_cast<SpawnOperation*>(that);
+                /* Take ownership */
+                auto op = std::unique_ptr<SpawnOperation>
+                        (reinterpret_cast<SpawnOperation*>(that));
         }
 
         static void run_in_thread_cb(GTask* task,
@@ -239,12 +241,14 @@ public:
         SpawnOperation operator=(SpawnOperation const&) = delete;
         SpawnOperation operator=(SpawnOperation&&) = delete;
 
-        void run_async(void *source_tag,
-                       GAsyncReadyCallback callback,
-                       void* user_data); /* takes ownership of @this ! */
+        static void run_async(std::unique_ptr<SpawnOperation> op,
+                              void *source_tag,
+                              GAsyncReadyCallback callback,
+                              void* user_data);
 
-        bool run_sync(GPid* pid,
-                      vte::glib::Error& error);
+        static bool run_sync(SpawnOperation& op,
+                             GPid* pid,
+                             vte::glib::Error& error);
 
 }; // class SpawnOperation
 
diff --git a/src/vtepty.cc b/src/vtepty.cc
index 7d5f2498..721a2d2b 100644
--- a/src/vtepty.cc
+++ b/src/vtepty.cc
@@ -684,7 +684,7 @@ try
                                             cancellable};
 
         auto err = vte::glib::Error{};
-        auto rv = op.run_sync(child_pid, err);
+        auto rv = vte::base::SpawnOperation::run_sync(op, child_pid, err);
         if (!rv)
                 err.propagate(error);
 
@@ -814,23 +814,24 @@ try
         g_warn_if_fail((spawn_flags & forbidden_spawn_flags()) == 0);
         spawn_flags = GSpawnFlags(spawn_flags & ~forbidden_spawn_flags());
 
-        auto op = new vte::base::SpawnOperation{spawn_context_from_args(pty,
-                                                                        working_directory,
-                                                                        argv,
-                                                                        envv,
-                                                                        fds, n_fds,
-                                                                        fd_map_to, n_fd_map_to,
-                                                                        spawn_flags,
-                                                                        child_setup,
-                                                                        child_setup_data,
-                                                                        child_setup_data_destroy),
-                                                timeout,
-                                                cancellable};
-
-        /* takes ownership of @op */
-        op->run_async((void*)vte_pty_spawn_async, /* tag */
-                      callback,
-                      user_data);
+        auto op = std::make_unique<vte::base::SpawnOperation>
+                (spawn_context_from_args(pty,
+                                         working_directory,
+                                         argv,
+                                         envv,
+                                         fds, n_fds,
+                                         fd_map_to, n_fd_map_to,
+                                         spawn_flags,
+                                         child_setup,
+                                         child_setup_data,
+                                         child_setup_data_destroy),
+                 timeout,
+                 cancellable);
+
+        vte::base::SpawnOperation::run_async(std::move(op),
+                                             (void*)vte_pty_spawn_async, /* tag */
+                                             callback,
+                                             user_data);
 }
 catch (...)
 {