+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
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):
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,
if (!message)
{
+ _dbus_verbose ("No memory to create reply to activate message\n");
BUS_SET_OOM (error);
return FALSE;
}
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;
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;
}
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;
}
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;
}
}
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;
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);
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);
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);
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);
}
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]
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;
}
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;
}
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;
{
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);
_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))
{
va_end (args);
}
+static dbus_bool_t verbose_initted = FALSE;
+
/**
* Prints a warning message to stderr
* if the user has enabled verbose mode.
{
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
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;
}
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.
#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(...)
# 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);
*/
#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.
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 */
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);
{
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
_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 ();
}
}
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]);