From 7b8b76eefc96f7d5802fa6a5319e90786e17a451 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 21 Jan 2011 18:24:19 +0000 Subject: [PATCH] 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 --- bus/activation.c | 34 +++++++++++----------------------- dbus/dbus-spawn-win.c | 18 ++++++++++++++++++ dbus/dbus-spawn.c | 19 +++++++++++++++++++ dbus/dbus-spawn.h | 6 ++++++ 4 files changed, 54 insertions(+), 23 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index 2349612..ec4c471 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -1304,22 +1304,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. @@ -1330,15 +1324,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; @@ -1393,8 +1379,6 @@ babysitter_watch_callback (DBusWatch *watch, } _dbus_babysitter_unref (babysitter); - - return retval; } static dbus_bool_t @@ -1403,9 +1387,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 @@ -2072,6 +2056,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 dfeb44f..df07372 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 5654380..a4652a3 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 5af54b7..a8814fb 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); -- 1.7.2.3