From eb63ba5039c8afe61210cf2b217ec75b4a86356e Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Fri, 11 Apr 2003 03:05:58 +0000 Subject: 2003-04-10 Havoc Pennington * dbus/dbus-spawn.c (_dbus_spawn_async_with_babysitter): move all the possible parent failures before we fork, so that we don't fail to create a babysitter after creating the child. * bus/activation.c (bus_activation_activate_service): kill child if we don't successfully complete the activation. --- dbus/dbus-connection.c | 4 +-- dbus/dbus-internals.c | 19 ++++++++++-- dbus/dbus-internals.h | 12 +++++--- dbus/dbus-spawn.c | 83 ++++++++++++++++++++++++++++++-------------------- 4 files changed, 75 insertions(+), 43 deletions(-) (limited to 'dbus') diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 5a6277de..9595f48a 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -1527,9 +1527,9 @@ dbus_connection_send_with_reply_and_block (DBusConnection *connection, _dbus_get_current_time (&tv_sec, &tv_usec); if (tv_sec < start_tv_sec) - ; /* clock set backward, bail out */ + _dbus_verbose ("dbus_connection_send_with_reply_and_block(): clock set backward\n"); else if (connection->disconnect_message_link == NULL) - ; /* we're disconnected, bail out */ + _dbus_verbose ("dbus_connection_send_with_reply_and_block(): disconnected\n"); else if (tv_sec < end_tv_sec || (tv_sec == end_tv_sec && tv_usec < end_tv_usec)) { diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index 230c8efe..30f47f00 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -174,6 +174,8 @@ _dbus_warn (const char *format, va_end (args); } +static dbus_bool_t verbose_initted = FALSE; + /** * Prints a warning message to stderr * if the user has enabled verbose mode. @@ -188,7 +190,6 @@ _dbus_verbose_real (const char *format, { va_list args; static dbus_bool_t verbose = TRUE; - static dbus_bool_t initted = FALSE; static unsigned long pid; /* things are written a bit oddly here so that @@ -198,11 +199,11 @@ _dbus_verbose_real (const char *format, if (!verbose) return; - if (!initted) + if (!verbose_initted) { verbose = _dbus_getenv ("DBUS_VERBOSE") != NULL; pid = _dbus_getpid (); - initted = TRUE; + verbose_initted = TRUE; if (!verbose) return; } @@ -214,6 +215,18 @@ _dbus_verbose_real (const char *format, va_end (args); } +/** + * Reinitializes the verbose logging code, used + * as a hack in dbus-spawn.c so that a child + * process re-reads its pid + * + */ +void +_dbus_verbose_reset_real (void) +{ + verbose_initted = FALSE; +} + /** * Duplicates a string. Result must be freed with * dbus_free(). Returns #NULL if memory allocation fails. diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index b6222fae..b0f41278 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -53,14 +53,15 @@ DBUS_BEGIN_DECLS; #define _DBUS_GNUC_NORETURN #endif /* !__GNUC__ */ -void _dbus_warn (const char *format, - ...) _DBUS_GNUC_PRINTF (1, 2); -void _dbus_verbose_real (const char *format, - ...) _DBUS_GNUC_PRINTF (1, 2); - +void _dbus_warn (const char *format, + ...) _DBUS_GNUC_PRINTF (1, 2); +void _dbus_verbose_real (const char *format, + ...) _DBUS_GNUC_PRINTF (1, 2); +void _dbus_verbose_reset_real (void); #ifdef DBUS_ENABLE_VERBOSE_MODE # define _dbus_verbose _dbus_verbose_real +# define _dbus_verbose_reset _dbus_verbose_reset_real #else # ifdef HAVE_ISO_VARARGS # define _dbus_verbose(...) @@ -69,6 +70,7 @@ void _dbus_verbose_real (const char *format, # else # error "This compiler does not support varargs macros and thus verbose mode can't be disabled meaningfully" # endif +# define _dbus_verbose_reset() #endif /* !DBUS_ENABLE_VERBOSE_MODE */ const char* _dbus_strerror (int error_number); diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c index 0e08cd78..c11dba21 100644 --- a/dbus/dbus-spawn.c +++ b/dbus/dbus-spawn.c @@ -544,7 +544,6 @@ babysitter_iteration (DBusBabysitter *sitter, */ #define LIVE_CHILDREN(sitter) ((sitter)->socket_to_babysitter >= 0 || (sitter)->error_pipe_from_child >= 0) - /** * Blocks until the babysitter process gives us the PID of the spawned grandchild, * then kills the spawned grandchild. @@ -559,6 +558,9 @@ _dbus_babysitter_kill_child (DBusBabysitter *sitter) sitter->grandchild_pid == -1) babysitter_iteration (sitter, TRUE); + _dbus_verbose ("Got child PID %ld for killing\n", + (long) sitter->grandchild_pid); + if (sitter->grandchild_pid == -1) return; /* child is already dead, or we're so hosed we'll never recover */ @@ -826,6 +828,10 @@ do_exec (int child_err_report_fd, int i, max_open; #endif + _dbus_verbose_reset (); + _dbus_verbose ("Child process has PID %lu\n", + _dbus_getpid ()); + if (child_setup) (* child_setup) (user_data); @@ -921,6 +927,11 @@ babysit (pid_t grandchild_pid, { int sigchld_pipe[2]; + /* We don't exec, so we keep parent state, such as the pid that + * _dbus_verbose() uses. Reset the pid here. + */ + _dbus_verbose_reset (); + /* I thought SIGCHLD would just wake up the poll, but * that didn't seem to work, so added this pipe. * Probably the pipe is more likely to work on busted @@ -1029,6 +1040,43 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, _dbus_fd_set_close_on_exec (babysitter_pipe[0]); _dbus_fd_set_close_on_exec (babysitter_pipe[1]); + + /* Setting up the babysitter is only useful in the parent, + * but we don't want to run out of memory and fail + * after we've already forked, since then we'd leak + * child processes everywhere. + */ + sitter->error_watch = _dbus_watch_new (child_err_report_pipe[READ_END], + DBUS_WATCH_READABLE, + TRUE); + if (sitter->error_watch == NULL) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); + goto cleanup_and_fail; + } + + if (!_dbus_watch_list_add_watch (sitter->watches, sitter->error_watch)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); + goto cleanup_and_fail; + } + + sitter->sitter_watch = _dbus_watch_new (babysitter_pipe[0], + DBUS_WATCH_READABLE, + TRUE); + if (sitter->sitter_watch == NULL) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); + goto cleanup_and_fail; + } + + if (!_dbus_watch_list_add_watch (sitter->watches, sitter->sitter_watch)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); + goto cleanup_and_fail; + } + + _DBUS_ASSERT_ERROR_IS_CLEAR (error); pid = fork (); @@ -1077,38 +1125,7 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, } } else - { - /* Parent */ - sitter->error_watch = _dbus_watch_new (child_err_report_pipe[READ_END], - DBUS_WATCH_READABLE, - TRUE); - if (sitter->error_watch == NULL) - { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); - goto cleanup_and_fail; - } - - if (!_dbus_watch_list_add_watch (sitter->watches, sitter->error_watch)) - { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); - goto cleanup_and_fail; - } - - sitter->sitter_watch = _dbus_watch_new (babysitter_pipe[0], - DBUS_WATCH_READABLE, - TRUE); - if (sitter->sitter_watch == NULL) - { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); - goto cleanup_and_fail; - } - - if (!_dbus_watch_list_add_watch (sitter->watches, sitter->sitter_watch)) - { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); - goto cleanup_and_fail; - } - + { /* Close the uncared-about ends of the pipes */ close_and_invalidate (&child_err_report_pipe[WRITE_END]); close_and_invalidate (&babysitter_pipe[1]); -- cgit