2003-04-10 Havoc Pennington <hp@pobox.com>

* 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.
This commit is contained in:
Havoc Pennington 2003-04-11 03:05:58 +00:00
parent 6be547d32f
commit eb63ba5039
7 changed files with 185 additions and 56 deletions

View file

@ -1,3 +1,12 @@
2003-04-10 Havoc Pennington <hp@pobox.com>
* 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.
2003-04-10 Havoc Pennington <hp@redhat.com>
* dbus/dbus-connection.c (dbus_connection_flush): don't spin on
@ -9,7 +18,7 @@
pending activation if the function fails.
* dbus/dbus-list.c (_dbus_list_insert_before_link)
(_dbus_list_insert_after_link): new code to facilitate
(_dbus_list_insert_after_link): new code to facilitate
services.c fixes
* dbus/dbus-hash.c (_dbus_hash_table_insert_string_preallocated):

View file

@ -779,6 +779,45 @@ pending_activation_timed_out (void *data)
return TRUE;
}
static void
cancel_pending (void *data)
{
BusPendingActivation *pending_activation = data;
_dbus_verbose ("Canceling pending activation of %s\n",
pending_activation->service_name);
if (pending_activation->babysitter)
_dbus_babysitter_kill_child (pending_activation->babysitter);
_dbus_hash_table_remove_string (pending_activation->activation->pending_activations,
pending_activation->service_name);
}
static void
free_pending_cancel_data (void *data)
{
BusPendingActivation *pending_activation = data;
bus_pending_activation_unref (pending_activation);
}
static dbus_bool_t
add_cancel_pending_to_transaction (BusTransaction *transaction,
BusPendingActivation *pending_activation)
{
if (!bus_transaction_add_cancel_hook (transaction, cancel_pending,
pending_activation,
free_pending_cancel_data))
return FALSE;
bus_pending_activation_ref (pending_activation);
_dbus_verbose ("Saved pending activation to be canceled if the transaction fails\n");
return TRUE;
}
dbus_bool_t
bus_activation_activate_service (BusActivation *activation,
DBusConnection *connection,
@ -817,6 +856,7 @@ bus_activation_activate_service (BusActivation *activation,
if (!message)
{
_dbus_verbose ("No memory to create reply to activate message\n");
BUS_SET_OOM (error);
return FALSE;
}
@ -826,6 +866,7 @@ bus_activation_activate_service (BusActivation *activation,
DBUS_TYPE_UINT32, DBUS_ACTIVATION_REPLY_ALREADY_ACTIVE,
0))
{
_dbus_verbose ("No memory to set args of reply to activate message\n");
BUS_SET_OOM (error);
dbus_message_unref (message);
return FALSE;
@ -834,7 +875,10 @@ bus_activation_activate_service (BusActivation *activation,
retval = bus_transaction_send_message (transaction, connection, message);
dbus_message_unref (message);
if (!retval)
BUS_SET_OOM (error);
{
_dbus_verbose ("Failed to send reply\n");
BUS_SET_OOM (error);
}
return retval;
}
@ -842,6 +886,7 @@ bus_activation_activate_service (BusActivation *activation,
pending_activation_entry = dbus_new0 (BusPendingActivationEntry, 1);
if (!pending_activation_entry)
{
_dbus_verbose ("Failed to create pending activation entry\n");
BUS_SET_OOM (error);
return FALSE;
}
@ -855,11 +900,15 @@ bus_activation_activate_service (BusActivation *activation,
pending_activation = _dbus_hash_table_lookup_string (activation->pending_activations, service_name);
if (pending_activation)
{
/* FIXME security - a client could keep sending activations over and
* over, growing this queue.
*/
if (!_dbus_list_append (&pending_activation->entries, pending_activation_entry))
{
_dbus_verbose ("Failed to append a new entry to pending activation\n");
BUS_SET_OOM (error);
bus_pending_activation_entry_free (pending_activation_entry);
return FALSE;
}
}
@ -868,6 +917,8 @@ bus_activation_activate_service (BusActivation *activation,
pending_activation = dbus_new0 (BusPendingActivation, 1);
if (!pending_activation)
{
_dbus_verbose ("Failed to create pending activation\n");
BUS_SET_OOM (error);
bus_pending_activation_entry_free (pending_activation_entry);
return FALSE;
@ -879,6 +930,8 @@ bus_activation_activate_service (BusActivation *activation,
pending_activation->service_name = _dbus_strdup (service_name);
if (!pending_activation->service_name)
{
_dbus_verbose ("Failed to copy service name for pending activation\n");
BUS_SET_OOM (error);
bus_pending_activation_unref (pending_activation);
bus_pending_activation_entry_free (pending_activation_entry);
@ -892,6 +945,8 @@ bus_activation_activate_service (BusActivation *activation,
NULL);
if (!pending_activation->timeout)
{
_dbus_verbose ("Failed to create timeout for pending activation\n");
BUS_SET_OOM (error);
bus_pending_activation_unref (pending_activation);
bus_pending_activation_entry_free (pending_activation_entry);
@ -904,6 +959,8 @@ bus_activation_activate_service (BusActivation *activation,
pending_activation,
NULL))
{
_dbus_verbose ("Failed to add timeout for pending activation\n");
BUS_SET_OOM (error);
bus_pending_activation_unref (pending_activation);
bus_pending_activation_entry_free (pending_activation_entry);
@ -914,6 +971,8 @@ bus_activation_activate_service (BusActivation *activation,
if (!_dbus_list_append (&pending_activation->entries, pending_activation_entry))
{
_dbus_verbose ("Failed to add entry to just-created pending activation\n");
BUS_SET_OOM (error);
bus_pending_activation_unref (pending_activation);
bus_pending_activation_entry_free (pending_activation_entry);
@ -921,13 +980,25 @@ bus_activation_activate_service (BusActivation *activation,
}
if (!_dbus_hash_table_insert_string (activation->pending_activations,
pending_activation->service_name, pending_activation))
pending_activation->service_name,
pending_activation))
{
_dbus_verbose ("Failed to put pending activation in hash table\n");
BUS_SET_OOM (error);
bus_pending_activation_unref (pending_activation);
return FALSE;
}
}
if (!add_cancel_pending_to_transaction (transaction, pending_activation))
{
_dbus_verbose ("Failed to add pending activation cancel hook to transaction\n");
BUS_SET_OOM (error);
_dbus_hash_table_remove_string (activation->pending_activations,
pending_activation->service_name);
return FALSE;
}
/* FIXME we need to support a full command line, not just a single
* argv[0]
@ -941,8 +1012,8 @@ bus_activation_activate_service (BusActivation *activation,
child_setup, activation,
error))
{
_dbus_hash_table_remove_string (activation->pending_activations,
pending_activation->service_name);
_dbus_verbose ("Failed to spawn child\n");
_DBUS_ASSERT_ERROR_IS_SET (error);
return FALSE;
}
@ -956,9 +1027,7 @@ bus_activation_activate_service (BusActivation *activation,
NULL))
{
BUS_SET_OOM (error);
_dbus_hash_table_remove_string (activation->pending_activations,
pending_activation->service_name);
_dbus_verbose ("Failed to set babysitter watch functions\n");
return FALSE;
}

View file

@ -584,13 +584,21 @@ bus_driver_handle_activate_service (DBusConnection *connection,
DBUS_TYPE_STRING, &name,
DBUS_TYPE_UINT32, &flags,
0))
return FALSE;
{
_DBUS_ASSERT_ERROR_IS_SET (error);
_dbus_verbose ("No memory to get arguments to ActivateService\n");
return FALSE;
}
retval = FALSE;
if (!bus_activation_activate_service (activation, connection, transaction,
message, name, error))
goto out;
{
_DBUS_ASSERT_ERROR_IS_SET (error);
_dbus_verbose ("bus_activation_activate_service() failed\n");
goto out;
}
retval = TRUE;
@ -650,15 +658,26 @@ bus_driver_handle_message (DBusConnection *connection,
{
if (strcmp (message_handlers[i].name, name) == 0)
{
_dbus_verbose ("Running driver handler for %s\n", name);
if ((* message_handlers[i].handler) (connection, transaction, message, error))
return TRUE;
{
_DBUS_ASSERT_ERROR_IS_CLEAR (error);
_dbus_verbose ("Driver handler succeeded\n");
return TRUE;
}
else
return FALSE;
{
_DBUS_ASSERT_ERROR_IS_SET (error);
_dbus_verbose ("Driver handler returned failure\n");
return FALSE;
}
}
++i;
}
_dbus_verbose ("No driver handler for %s\n", name);
dbus_set_error (error, DBUS_ERROR_UNKNOWN_MESSAGE,
"%s does not understand message %s",
DBUS_SERVICE_DBUS, name);

View file

@ -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))
{

View file

@ -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.

View file

@ -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);

View file

@ -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]);