bus-activation: separate the "finished" callback from the watch callback

This has been marked as broken since 2003...

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=33342
Reviewed-by: Thiago Macieira <thiago@kde.org>

4 files changed, 54 insertions, 23 deletions

diff --git a/bus/activation.c b/bus/activation.c
index 7ce463c3..d5424a7c 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -1323,22 +1323,16 @@ handle_servicehelper_exit_error (int        exit_code,
     }
 }
 
-static dbus_bool_t
-babysitter_watch_callback (DBusWatch     *watch,
-                           unsigned int   condition,
-                           void          *data)
+static void
+pending_activation_finished_cb (DBusBabysitter *babysitter,
+                                void           *data)
 {
   BusPendingActivation *pending_activation = data;
-  dbus_bool_t retval;
-  DBusBabysitter *babysitter;
   dbus_bool_t uses_servicehelper;
 
-  babysitter = pending_activation->babysitter;
-
+  _dbus_assert (babysitter == pending_activation->babysitter);
   _dbus_babysitter_ref (babysitter);
 
-  retval = dbus_watch_handle (watch, condition);
-
   /* There are two major cases here; are we the system bus or the session?  Here this
    * is distinguished by whether or not we use a setuid helper launcher.  With the launch helper,
    * some process exit codes are meaningful, processed by handle_servicehelper_exit_error.
@@ -1349,15 +1343,7 @@ babysitter_watch_callback (DBusWatch     *watch,
    */
   uses_servicehelper = bus_context_get_servicehelper (pending_activation->activation->context) != NULL;
 
-  /* FIXME this is broken in the same way that
-   * connection watches used to be; there should be
-   * a separate callback for status change, instead
-   * of doing "if we handled a watch status might
-   * have changed"
-   *
-   * Fixing this lets us move dbus_watch_handle
-   * calls into dbus-mainloop.c
-   */
+  /* strictly speaking this is redundant with the check in dbus-spawn now */
   if (_dbus_babysitter_get_child_exited (babysitter))
     {
       DBusError error;
@@ -1417,8 +1403,6 @@ babysitter_watch_callback (DBusWatch     *watch,
     }
 
   _dbus_babysitter_unref (babysitter);
-
-  return retval;
 }
 
 static dbus_bool_t
@@ -1427,9 +1411,9 @@ add_babysitter_watch (DBusWatch      *watch,
 {
   BusPendingActivation *pending_activation = data;
 
-  return _dbus_loop_add_watch_full (
+  return _dbus_loop_add_watch (
       bus_context_get_loop (pending_activation->activation->context),
-      watch, babysitter_watch_callback, pending_activation, NULL);
+      watch);
 }
 
 static void
@@ -2119,6 +2103,10 @@ bus_activation_activate_service (BusActivation  *activation,
 
   _dbus_assert (pending_activation->babysitter != NULL);
 
+  _dbus_babysitter_set_result_function (pending_activation->babysitter,
+                                        pending_activation_finished_cb,
+                                        pending_activation);
+
   if (!_dbus_babysitter_set_watch_functions (pending_activation->babysitter,
                                              add_babysitter_watch,
                                              remove_babysitter_watch,
diff --git a/dbus/dbus-spawn-win.c b/dbus/dbus-spawn-win.c
index dfeb44f6..df073722 100644
--- a/dbus/dbus-spawn-win.c
+++ b/dbus/dbus-spawn-win.c
@@ -81,6 +81,8 @@ struct DBusBabysitter
 
     DBusWatchList *watches;
     DBusWatch *sitter_watch;
+    DBusBabysitterFinishedFunc finished_cb;
+    void *finished_data;
 
     dbus_bool_t have_spawn_errno;
     int spawn_errno;
@@ -392,6 +394,13 @@ handle_watch (DBusWatch       *watch,
   close_socket_to_babysitter (sitter);
   PING();
 
+  if (_dbus_babysitter_get_child_exited (sitter) &&
+      sitter->finished_cb != NULL)
+    {
+      sitter->finished_cb (sitter, sitter->finished_data);
+      sitter->finished_cb = NULL;
+    }
+
   return TRUE;
 }
 
@@ -735,6 +744,15 @@ out0:
   return FALSE;
 }
 
+void
+_dbus_babysitter_set_result_function  (DBusBabysitter             *sitter,
+                                       DBusBabysitterFinishedFunc  finished,
+                                       void                       *user_data)
+{
+  sitter->finished_cb = finished;
+  sitter->finished_data = user_data;
+}
+
 #ifdef DBUS_BUILD_TESTS
 
 #define LIVE_CHILDREN(sitter) ((sitter)->child_handle != NULL)
diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c
index 5654380d..a4652a3e 100644
--- a/dbus/dbus-spawn.c
+++ b/dbus/dbus-spawn.c
@@ -205,6 +205,9 @@ struct DBusBabysitter
   DBusWatch *error_watch; /**< Error pipe watch */
   DBusWatch *sitter_watch; /**< Sitter pipe watch */
 
+  DBusBabysitterFinishedFunc finished_cb;
+  void *finished_data;
+
   int errnum; /**< Error number */
   int status; /**< Exit status code */
   unsigned int have_child_status : 1; /**< True if child status has been reaped */
@@ -787,6 +790,13 @@ handle_watch (DBusWatch       *watch,
   _dbus_assert (sitter->socket_to_babysitter != -1 || sitter->sitter_watch == NULL);
   _dbus_assert (sitter->error_pipe_from_child != -1 || sitter->error_watch == NULL);
 
+  if (_dbus_babysitter_get_child_exited (sitter) &&
+      sitter->finished_cb != NULL)
+    {
+      sitter->finished_cb (sitter, sitter->finished_data);
+      sitter->finished_cb = NULL;
+    }
+
   _dbus_babysitter_unref (sitter);
   return TRUE;
 }
@@ -1298,6 +1308,15 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter          **sitter_p,
   return FALSE;
 }
 
+void
+_dbus_babysitter_set_result_function  (DBusBabysitter             *sitter,
+                                       DBusBabysitterFinishedFunc  finished,
+                                       void                       *user_data)
+{
+  sitter->finished_cb = finished;
+  sitter->finished_data = user_data;
+}
+
 /** @} */
 
 #ifdef DBUS_BUILD_TESTS
diff --git a/dbus/dbus-spawn.h b/dbus/dbus-spawn.h
index 5af54b72..a8814fb9 100644
--- a/dbus/dbus-spawn.h
+++ b/dbus/dbus-spawn.h
@@ -35,12 +35,18 @@ typedef void (* DBusSpawnChildSetupFunc) (void *user_data);
 
 typedef struct DBusBabysitter DBusBabysitter;
 
+typedef void (* DBusBabysitterFinishedFunc) (DBusBabysitter *sitter,
+                                             void           *user_data);
+
 dbus_bool_t _dbus_spawn_async_with_babysitter     (DBusBabysitter           **sitter_p,
                                                    char                     **argv,
                                                    char                     **env,
                                                    DBusSpawnChildSetupFunc    child_setup,
                                                    void                      *user_data,
                                                    DBusError                 *error);
+void        _dbus_babysitter_set_result_function  (DBusBabysitter            *sitter,
+                                                   DBusBabysitterFinishedFunc finished,
+                                                   void                      *user_data);
 DBusBabysitter* _dbus_babysitter_ref              (DBusBabysitter            *sitter);
 void        _dbus_babysitter_unref                (DBusBabysitter            *sitter);
 void        _dbus_babysitter_kill_child           (DBusBabysitter            *sitter);