diff options
| -rw-r--r-- | ChangeLog | 16 | ||||
| -rw-r--r-- | bus/activation.c | 265 | ||||
| -rw-r--r-- | bus/bus.c | 14 | ||||
| -rw-r--r-- | bus/bus.h | 1 | ||||
| -rw-r--r-- | bus/dispatch.c | 8 | ||||
| -rw-r--r-- | bus/loop.c | 155 | ||||
| -rw-r--r-- | dbus/dbus-errors.h | 1 | ||||
| -rw-r--r-- | dbus/dbus-spawn.c | 6 |
8 files changed, 396 insertions, 70 deletions
@@ -1,5 +1,21 @@ 2003-04-05 Havoc Pennington <hp@pobox.com> + * bus/loop.c (bus_loop_iterate): fix the timeout code, using + magic from GLib + + * dbus/dbus-spawn.c (_dbus_babysitter_unref): set sitter_pid + to -1 once we've reaped the babysitter + (_dbus_babysitter_handle_watch): do as much work as we can, not + just one go of it + + * bus/activation.c: add code using DBusBabysitter so that we + handle it when a service fails to start up properly. + (bus_activation_service_created): don't remove the activation + entries as we go, just let them get removed when we free the pending + activation. Unref reply messages after sending them. + +2003-04-05 Havoc Pennington <hp@pobox.com> + * test/decode-gcov.c (main): print per-directory stats in the report * Makefile.am (coverage-report.txt): don't include test/* in gcov stats diff --git a/bus/activation.c b/bus/activation.c index 2045baa2..54ddd948 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -29,6 +29,7 @@ #include <dbus/dbus-hash.h> #include <dbus/dbus-list.h> #include <dbus/dbus-spawn.h> +#include <dbus/dbus-timeout.h> #include <sys/types.h> #include <dirent.h> #include <errno.h> @@ -62,8 +63,12 @@ struct BusPendingActivationEntry typedef struct { + BusActivation *activation; char *service_name; DBusList *entries; + DBusBabysitter *babysitter; + DBusTimeout *timeout; + unsigned int timeout_added : 1; } BusPendingActivation; static void @@ -74,21 +79,53 @@ bus_pending_activation_entry_free (BusPendingActivationEntry *entry) if (entry->connection) dbus_connection_unref (entry->connection); - + dbus_free (entry); } static void -bus_pending_activation_free (BusPendingActivation *activation) +handle_timeout_callback (DBusTimeout *timeout, + void *data) +{ + BusPendingActivation *pending_activation = data; + + while (!dbus_timeout_handle (pending_activation->timeout)) + bus_wait_for_memory (); +} + +static void +bus_pending_activation_free (BusPendingActivation *pending_activation) { DBusList *link; - if (!activation) + if (pending_activation == NULL) /* hash table requires this */ return; - dbus_free (activation->service_name); + if (pending_activation->timeout_added) + { + bus_loop_remove_timeout (bus_context_get_loop (pending_activation->activation->context), + pending_activation->timeout, + handle_timeout_callback, pending_activation); + pending_activation->timeout_added = FALSE; + } + + if (pending_activation->timeout) + _dbus_timeout_unref (pending_activation->timeout); + + if (pending_activation->babysitter) + { + if (!_dbus_babysitter_set_watch_functions (pending_activation->babysitter, + NULL, NULL, NULL, + pending_activation->babysitter, + NULL)) + _dbus_assert_not_reached ("setting watch functions to NULL failed"); + + _dbus_babysitter_unref (pending_activation->babysitter); + } + + dbus_free (pending_activation->service_name); - link = _dbus_list_get_first_link (&activation->entries); + link = _dbus_list_get_first_link (&pending_activation->entries); while (link != NULL) { @@ -96,11 +133,11 @@ bus_pending_activation_free (BusPendingActivation *activation) bus_pending_activation_entry_free (entry); - link = _dbus_list_get_next_link (&activation->entries, link); + link = _dbus_list_get_next_link (&pending_activation->entries, link); } - _dbus_list_clear (&activation->entries); + _dbus_list_clear (&pending_activation->entries); - dbus_free (activation); + dbus_free (pending_activation); } static void @@ -478,11 +515,10 @@ bus_activation_service_created (BusActivation *activation, BUS_SET_OOM (error); goto error; } + + dbus_message_unref (message); } - bus_pending_activation_entry_free (entry); - - _dbus_list_remove_link (&pending_activation->entries, link); link = next; } @@ -495,6 +531,165 @@ bus_activation_service_created (BusActivation *activation, return FALSE; } +/** + * FIXME @todo the error messages here would ideally be preallocated + * so we don't need to allocate memory to send them. + * Using the usual tactic, prealloc an OOM message, then + * if we can't alloc the real error send the OOM error instead. + */ +static dbus_bool_t +try_send_activation_failure (BusPendingActivation *pending_activation, + const DBusError *how) +{ + BusActivation *activation; + DBusMessage *message; + DBusList *link; + BusTransaction *transaction; + + activation = pending_activation->activation; + + transaction = bus_transaction_new (activation->context); + if (transaction == NULL) + return FALSE; + + link = _dbus_list_get_first_link (&pending_activation->entries); + while (link != NULL) + { + BusPendingActivationEntry *entry = link->data; + DBusList *next = _dbus_list_get_next_link (&pending_activation->entries, link); + + if (dbus_connection_get_is_connected (entry->connection)) + { + message = dbus_message_new_error_reply (entry->activation_message, + how->name, + how->message); + if (!message) + goto error; + + if (!dbus_message_set_sender (message, DBUS_SERVICE_DBUS)) + { + dbus_message_unref (message); + goto error; + } + + if (!bus_transaction_send_message (transaction, entry->connection, message)) + { + dbus_message_unref (message); + goto error; + } + + dbus_message_unref (message); + } + + link = next; + } + + bus_transaction_execute_and_free (transaction); + + return TRUE; + + error: + if (transaction) + bus_transaction_cancel_and_free (transaction); + return FALSE; +} + +/** + * Free the pending activation and send an error message to all the + * connections that were waiting for it. + */ +static void +pending_activation_failed (BusPendingActivation *pending_activation, + const DBusError *how) +{ + /* FIXME use preallocated OOM messages instead of bus_wait_for_memory() */ + while (!try_send_activation_failure (pending_activation, how)) + bus_wait_for_memory (); + + /* Destroy this pending activation */ + _dbus_hash_table_remove_string (pending_activation->activation->pending_activations, + pending_activation->service_name); +} + +static dbus_bool_t +babysitter_watch_callback (DBusWatch *watch, + unsigned int condition, + void *data) +{ + BusPendingActivation *pending_activation = data; + dbus_bool_t retval; + DBusBabysitter *babysitter; + + babysitter = pending_activation->babysitter; + + _dbus_babysitter_ref (babysitter); + + retval = _dbus_babysitter_handle_watch (babysitter, watch, condition); + + if (_dbus_babysitter_get_child_exited (babysitter)) + { + DBusError error; + + dbus_error_init (&error); + _dbus_babysitter_set_child_exit_error (babysitter, &error); + + /* Destroys the pending activation */ + pending_activation_failed (pending_activation, &error); + + dbus_error_free (&error); + } + + _dbus_babysitter_unref (babysitter); + + return retval; +} + +static dbus_bool_t +add_babysitter_watch (DBusWatch *watch, + void *data) +{ + BusPendingActivation *pending_activation = data; + + return bus_loop_add_watch (bus_context_get_loop (pending_activation->activation->context), + watch, babysitter_watch_callback, pending_activation, + NULL); +} + +static void +remove_babysitter_watch (DBusWatch *watch, + void *data) +{ + BusPendingActivation *pending_activation = data; + + bus_loop_remove_watch (bus_context_get_loop (pending_activation->activation->context), + watch, babysitter_watch_callback, pending_activation); +} + +static dbus_bool_t +pending_activation_timed_out (void *data) +{ + BusPendingActivation *pending_activation = data; + DBusError error; + + /* Kill the spawned process, since it sucks + * (not sure this is what we want to do, but + * may as well try it for now) + */ + _dbus_babysitter_kill_child (pending_activation->babysitter); + + dbus_error_init (&error); + + dbus_set_error (&error, DBUS_ERROR_TIMED_OUT, + "Activation of %s timed out", + pending_activation->service_name); + + pending_activation_failed (pending_activation, &error); + + dbus_error_free (&error); + + return TRUE; +} + dbus_bool_t bus_activation_activate_service (BusActivation *activation, DBusConnection *connection, @@ -586,6 +781,9 @@ bus_activation_activate_service (BusActivation *activation, bus_pending_activation_entry_free (pending_activation_entry); return FALSE; } + + pending_activation->activation = activation; + pending_activation->service_name = _dbus_strdup (service_name); if (!pending_activation->service_name) { @@ -595,6 +793,33 @@ bus_activation_activate_service (BusActivation *activation, return FALSE; } + pending_activation->timeout = + _dbus_timeout_new (bus_context_get_activation_timeout (activation->context), + pending_activation_timed_out, + pending_activation, + NULL); + if (!pending_activation->timeout) + { + BUS_SET_OOM (error); + bus_pending_activation_free (pending_activation); + bus_pending_activation_entry_free (pending_activation_entry); + return FALSE; + } + + if (!bus_loop_add_timeout (bus_context_get_loop (activation->context), + pending_activation->timeout, + handle_timeout_callback, + pending_activation, + NULL)) + { + BUS_SET_OOM (error); + bus_pending_activation_free (pending_activation); + bus_pending_activation_entry_free (pending_activation_entry); + return FALSE; + } + + pending_activation->timeout_added = TRUE; + if (!_dbus_list_append (&pending_activation->entries, pending_activation_entry)) { BUS_SET_OOM (error); @@ -620,7 +845,7 @@ bus_activation_activate_service (BusActivation *activation, argv[0] = entry->exec; argv[1] = NULL; - if (!_dbus_spawn_async_with_babysitter (NULL, argv, + if (!_dbus_spawn_async_with_babysitter (&pending_activation->babysitter, argv, child_setup, activation, error)) { @@ -628,6 +853,22 @@ bus_activation_activate_service (BusActivation *activation, pending_activation->service_name); return FALSE; } + + _dbus_assert (pending_activation->babysitter != NULL); + + if (!_dbus_babysitter_set_watch_functions (pending_activation->babysitter, + add_babysitter_watch, + remove_babysitter_watch, + NULL, + pending_activation, + NULL)) + { + BUS_SET_OOM (error); + + _dbus_hash_table_remove_string (activation->pending_activations, + pending_activation->service_name); + return FALSE; + } return TRUE; } @@ -47,6 +47,7 @@ struct BusContext DBusList *mandatory_rules; /**< Mandatory policy rules */ DBusHashTable *rules_by_uid; /**< per-UID policy rules */ DBusHashTable *rules_by_gid; /**< per-GID policy rules */ + int activation_timeout; /**< How long to wait for an activation to time out */ }; static int server_data_slot = -1; @@ -333,6 +334,12 @@ bus_context_new (const DBusString *config_file, context->refcount = 1; +#ifdef DBUS_BUILD_TESTS + context->activation_timeout = 6000; /* 6/10 second */ /* FIXME */ +#else + context->activation_timeout = 10000; /* 10 seconds */ +#endif + context->loop = bus_loop_new (); if (context->loop == NULL) { @@ -853,3 +860,10 @@ bus_context_create_connection_policy (BusContext *context, bus_policy_unref (policy); return NULL; } + +int +bus_context_get_activation_timeout (BusContext *context) +{ + + return context->activation_timeout; +} @@ -54,5 +54,6 @@ dbus_bool_t bus_context_allow_user (BusContext *context, unsigned long uid); BusPolicy* bus_context_create_connection_policy (BusContext *context, DBusConnection *connection); +int bus_context_get_activation_timeout (BusContext *context); #endif /* BUS_BUS_H */ diff --git a/bus/dispatch.c b/bus/dispatch.c index 21d57ae1..6e3a61e3 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -1070,9 +1070,11 @@ check_existent_service_activation (BusContext *context, bus_test_run_everything (context); - /* now wait for the message bus to hear back from the activated service */ - bus_test_run_bus_loop (context); - + if (dbus_connection_get_dispatch_status (connection) == + DBUS_DISPATCH_COMPLETE) + /* now wait for the message bus to hear back from the activated service */ + bus_test_run_bus_loop (context); + /* and process everything again */ bus_test_run_everything (context); @@ -312,6 +312,90 @@ bus_loop_remove_timeout (BusLoop *loop, timeout, function, data); } +/* Convolutions from GLib, there really must be a better way + * to do this. + */ +static dbus_bool_t +check_timeout (unsigned long tv_sec, + unsigned long tv_usec, + TimeoutCallback *tcb, + int *timeout) +{ + long sec; + long msec; + unsigned long expiration_tv_sec; + unsigned long expiration_tv_usec; + long interval_seconds; + long interval_milliseconds; + int interval; + + interval = dbus_timeout_get_interval (tcb->timeout); + + interval_seconds = interval / 1000; + interval_milliseconds = interval - interval_seconds * 1000; + + expiration_tv_sec = tcb->last_tv_sec + interval_seconds; + expiration_tv_usec = tcb->last_tv_usec + interval_milliseconds * 1000; + if (expiration_tv_usec >= 1000000) + { + expiration_tv_usec -= 1000000; + expiration_tv_sec += 1; + } + + sec = expiration_tv_sec - tv_sec; + msec = (expiration_tv_usec - tv_usec) / 1000; + +#if 0 + printf ("Interval is %ld seconds %ld msecs\n", + interval_seconds, + interval_milliseconds); + printf ("Now is %lu seconds %lu usecs\n", + tv_sec, tv_usec); + printf ("Exp is %lu seconds %lu usecs\n", + expiration_tv_sec, expiration_tv_usec); + printf ("Pre-correction, remaining sec %ld msec %ld\n", sec, msec); +#endif + + /* We do the following in a rather convoluted fashion to deal with + * the fact that we don't have an integral type big enough to hold + * the difference of two timevals in millseconds. + */ + if (sec < 0 || (sec == 0 && msec < 0)) + msec = 0; + else + { + if (msec < 0) + { + msec += 1000; + sec -= 1; + } + + if (sec > interval_seconds || + (sec == interval_seconds && msec > interval_milliseconds)) + { + /* The system time has been set backwards, reset the timeout */ + tcb->last_tv_sec = tv_sec; + tcb->last_tv_usec = tv_usec; + + msec = MIN (_DBUS_INT_MAX, interval); + + _dbus_verbose ("System clock went backward\n"); + } + else + { + msec = MIN (_DBUS_INT_MAX, (unsigned int)msec + 1000 * (unsigned int)sec); + } + } + + *timeout = msec; + +#if 0 + printf ("Timeout expires in %d milliseconds\n", *timeout); +#endif + + return msec == 0; +} + /* Returns TRUE if we have any timeouts or ready file descriptors, * which is just used in test code as a debug hack */ @@ -447,34 +531,15 @@ bus_loop_iterate (BusLoop *loop, dbus_timeout_get_enabled (TIMEOUT_CALLBACK (cb)->timeout)) { TimeoutCallback *tcb = TIMEOUT_CALLBACK (cb); - unsigned long interval; - unsigned long elapsed; + int msecs_remaining; - if (tcb->last_tv_sec > tv_sec || - (tcb->last_tv_sec == tv_sec && - tcb->last_tv_usec > tv_usec)) - { - /* Clock went backward, pretend timeout - * was just installed. - */ - tcb->last_tv_sec = tv_sec; - tcb->last_tv_usec = tv_usec; - _dbus_verbose ("System clock went backward\n"); - } - - interval = dbus_timeout_get_interval (tcb->timeout); + check_timeout (tv_sec, tv_usec, tcb, &msecs_remaining); - elapsed = - (tv_sec - tcb->last_tv_sec) * 1000 + - (tv_usec - tcb->last_tv_usec) / 1000; - - if (interval < elapsed) - timeout = 0; - else if (timeout < 0) - timeout = interval - elapsed; + if (timeout < 0) + timeout = msecs_remaining; else - timeout = MIN (((unsigned long)timeout), interval - elapsed); - + timeout = MIN (msecs_remaining, timeout); + _dbus_assert (timeout >= 0); if (timeout == 0) @@ -486,7 +551,12 @@ bus_loop_iterate (BusLoop *loop, } if (!block) - timeout = 0; + { + timeout = 0; +#if 0 + printf ("timeout is 0 as we aren't blocking\n"); +#endif + } /* if a watch is OOM, don't wait longer than the OOM * wait to re-enable it @@ -522,41 +592,17 @@ bus_loop_iterate (BusLoop *loop, dbus_timeout_get_enabled (TIMEOUT_CALLBACK (cb)->timeout)) { TimeoutCallback *tcb = TIMEOUT_CALLBACK (cb); - unsigned long interval; - unsigned long elapsed; - - if (tcb->last_tv_sec > tv_sec || - (tcb->last_tv_sec == tv_sec && - tcb->last_tv_usec > tv_usec)) - { - /* Clock went backward, pretend timeout - * was just installed. - */ - tcb->last_tv_sec = tv_sec; - tcb->last_tv_usec = tv_usec; - _dbus_verbose ("System clock went backward\n"); - goto next_timeout; - } - - interval = dbus_timeout_get_interval (tcb->timeout); - - elapsed = - (tv_sec - tcb->last_tv_sec) * 1000 + - (tv_usec - tcb->last_tv_usec) / 1000; - -#if 0 - _dbus_verbose (" interval = %lu elapsed = %lu\n", - interval, elapsed); -#endif + int msecs_remaining; - if (interval <= elapsed) + if (check_timeout (tv_sec, tv_usec, + tcb, &msecs_remaining)) { /* Save last callback time and fire this timeout */ tcb->last_tv_sec = tv_sec; tcb->last_tv_usec = tv_usec; #if 0 - _dbus_verbose (" invoking timeout\n"); + printf (" invoking timeout\n"); #endif (* tcb->function) (tcb->timeout, @@ -564,7 +610,6 @@ bus_loop_iterate (BusLoop *loop, } } - next_timeout: link = next; } } diff --git a/dbus/dbus-errors.h b/dbus/dbus-errors.h index 59fa1e49..8d8e16ec 100644 --- a/dbus/dbus-errors.h +++ b/dbus/dbus-errors.h @@ -73,6 +73,7 @@ struct DBusError #define DBUS_ERROR_INVALID_ARGS "org.freedesktop.DBus.Error.InvalidArgs" #define DBUS_ERROR_FILE_NOT_FOUND "org.freedesktop.DBus.Error.FileNotFound" #define DBUS_ERROR_UNKNOWN_MESSAGE "org.freedesktop.DBus.Error.UnknownMessage" +#define DBUS_ERROR_TIMED_OUT "org.freedesktop.DBus.Error.TimedOut" void dbus_error_init (DBusError *error); void dbus_error_free (DBusError *error); diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c index 5ced84fc..5fa2d5e8 100644 --- a/dbus/dbus-spawn.c +++ b/dbus/dbus-spawn.c @@ -293,6 +293,8 @@ _dbus_babysitter_unref (DBusBabysitter *sitter) else _dbus_verbose ("Babysitter exited abnormally\n"); } + + sitter->sitter_pid = -1; } if (sitter->error_watch) @@ -699,6 +701,10 @@ _dbus_babysitter_handle_watch (DBusBabysitter *sitter, handle_error_pipe (sitter, revents); else if (fd == sitter->socket_to_babysitter) handle_babysitter_socket (sitter, revents); + + while (LIVE_CHILDREN (sitter) && + babysitter_iteration (sitter, FALSE)) + ; return TRUE; } |