From d00d4a6b41d65ff1058e5e152736d0ba24fce2f5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 19 Dec 2010 13:00:44 -0500 Subject: [PATCH] Scope autolaunch to requesting client, don't connect to X server Previously, when logging in via ssh with X forwarding, since on most operating systems there is no DBUS_SESSION_BUS_ADDRESS, we autolaunch a bus. In this is that dbus-launch will connect to the remote X server and scope itself to that, which will cause ssh to refuse to exit because there are still clients. Basically: ssh -X remotehost firefox & Now, scoping of the bus is a long debated mess, but there is a reasonable fix for the ssh -X case; we scope the bus lifetime to the requesting client, not to the X server. This patch adds --exit-after-clients-exit option to the bus, which will cause the bus, after an initial connection is made once, to exit after it has no connections. dbus-launch --autolaunch is modified to use this. Now, in the Firefox case what's actually going on is that it talks to GConf, which was recently modified to scope to DBus, bus since no bus is available it autolaunches one. The resulting gconfd-2 process hangs around after ssh exits, but will eventually time out. Possible regressions: * Previously dbus-launch looked at the X server to try to find a bus address; this worked in the "local" (i.e. same uid-kernel) case where one manually sets: DISPLAY=:0 somedbusapp A specific example of this case is the legacy Unix getty. But I don't see this as a big problem; there are other environment variables, and really, the right fix for this is to have the OS default to single session. https://bugs.freedesktop.org/show_bug.cgi?id=32509 --- bus/bus.c | 39 +++++++++++++++++++++++- bus/bus.h | 3 ++ bus/connection.c | 35 ++++++++++++++++++++- bus/connection.h | 3 ++ bus/main.c | 7 +++- bus/test.c | 3 +- tools/dbus-launch.c | 85 +++++--------------------------------------------- 7 files changed, 94 insertions(+), 81 deletions(-) diff --git a/bus/bus.c b/bus/bus.c index e8276af..2ec56e1 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -64,6 +64,11 @@ struct BusContext unsigned int keep_umask : 1; unsigned int allow_anonymous : 1; unsigned int systemd_activation : 1; + unsigned int exit_after_clients_exit : 1; + + /* Set for exit_after_clients_exit mode after first client + connects */ + unsigned int got_one_connection : 1; }; static dbus_int32_t server_data_slot = -1; @@ -281,6 +286,7 @@ process_config_first_time_only (BusContext *context, BusConfigParser *parser, const DBusString *address, dbus_bool_t systemd_activation, + dbus_bool_t exit_after_clients_exit, DBusError *error) { DBusString log_prefix; @@ -298,6 +304,7 @@ process_config_first_time_only (BusContext *context, auth_mechanisms = NULL; context->systemd_activation = systemd_activation; + context->exit_after_clients_exit = exit_after_clients_exit; /* Check for an existing pid file. Of course this is a race; * we'd have to use fcntl() locks on the pid file to @@ -679,6 +686,7 @@ bus_context_new (const DBusString *config_file, DBusPipe *print_pid_pipe, const DBusString *address, dbus_bool_t systemd_activation, + dbus_bool_t exit_after_clients_exit, DBusError *error) { DBusString log_prefix; @@ -733,7 +741,10 @@ bus_context_new (const DBusString *config_file, goto failed; } - if (!process_config_first_time_only (context, parser, address, systemd_activation, error)) + if (!process_config_first_time_only (context, parser, address, + systemd_activation, + exit_after_clients_exit, + error)) { _DBUS_ASSERT_ERROR_IS_SET (error); goto failed; @@ -1244,6 +1255,32 @@ bus_context_get_reply_timeout (BusContext *context) return context->limits.reply_timeout; } +/** + * Call this function after a client connects or disconnects. + * Intended for handling the auto-exit functionality, where the bus + * can optionally exit if there are no clients. + * + * @param context the context + */ +void +bus_context_on_client_state_changed (BusContext *context) +{ + dbus_bool_t have_clients; + int connection_count; + + if (!context->exit_after_clients_exit) + return; + + connection_count = bus_connections_get_n_completed (context->connections) + + bus_connections_get_n_incomplete (context->connections); + have_clients = connection_count > 0; + if (have_clients && !context->got_one_connection) + context->got_one_connection = TRUE; + else if (!have_clients && context->got_one_connection) + _dbus_exit (0); +} + + void bus_context_log (BusContext *context, DBusSystemLogSeverity severity, const char *msg, ...) _DBUS_GNUC_PRINTF (3, 4); diff --git a/bus/bus.h b/bus/bus.h index ebef17c..b41a2e7 100644 --- a/bus/bus.h +++ b/bus/bus.h @@ -77,6 +77,7 @@ BusContext* bus_context_new (const DBusStri DBusPipe *print_pid_pipe, const DBusString *address, dbus_bool_t systemd_activation, + dbus_bool_t exit_after_clients_exit, DBusError *error); dbus_bool_t bus_context_reload_config (BusContext *context, DBusError *error); @@ -100,6 +101,8 @@ dbus_bool_t bus_context_allow_windows_user (BusContext const char *windows_sid); BusPolicy* bus_context_get_policy (BusContext *context); +void bus_context_on_client_state_changed (BusContext *context); + BusClientPolicy* bus_context_create_client_policy (BusContext *context, DBusConnection *connection, DBusError *error); diff --git a/bus/connection.c b/bus/connection.c index 8e7d222..40da161 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -172,6 +172,7 @@ adjust_connections_for_uid (BusConnections *connections, void bus_connection_disconnected (DBusConnection *connection) { + BusContext *context; BusConnectionData *d; BusService *service; BusMatchmaker *matchmaker; @@ -179,13 +180,15 @@ bus_connection_disconnected (DBusConnection *connection) d = BUS_CONNECTION_DATA (connection); _dbus_assert (d != NULL); + context = d->connections->context; + _dbus_verbose ("%s disconnected, dropping all service ownership and releasing\n", d->name ? d->name : "(inactive)"); /* Delete our match rules */ if (d->n_match_rules > 0) { - matchmaker = bus_context_get_matchmaker (d->connections->context); + matchmaker = bus_context_get_matchmaker (context); bus_matchmaker_disconnected (matchmaker, connection); } @@ -205,7 +208,7 @@ bus_connection_disconnected (DBusConnection *connection) dbus_error_init (&error); - while ((transaction = bus_transaction_new (d->connections->context)) == NULL) + while ((transaction = bus_transaction_new (context)) == NULL) _dbus_wait_for_memory (); if (!bus_service_remove_owner (service, connection, @@ -292,6 +295,8 @@ bus_connection_disconnected (DBusConnection *connection) NULL, NULL); dbus_connection_unref (connection); + + bus_context_on_client_state_changed (context); } static dbus_bool_t @@ -730,6 +735,8 @@ bus_connections_setup_connection (BusConnections *connections, */ dbus_connection_close (connections->incomplete->data); } + + bus_context_on_client_state_changed (d->connections->context); retval = TRUE; @@ -1017,6 +1024,28 @@ bus_connections_foreach (BusConnections *connections, foreach_inactive (connections, function, data); } +/** + * Return the number of pending but not yet completed connections. + * + * @param connections connections + */ +int +bus_connections_get_n_incomplete (BusConnections *connections) +{ + return connections->n_incomplete; +} + +/** + * Return the number of active, completed connections. + * + * @param connections connections + */ +int +bus_connections_get_n_completed (BusConnections *connections) +{ + return connections->n_completed; +} + BusContext* bus_connections_get_context (BusConnections *connections) { @@ -1407,6 +1436,8 @@ bus_connection_complete (DBusConnection *connection, /* See if we can remove the timeout */ bus_connections_expire_incomplete (d->connections); + bus_context_on_client_state_changed (d->connections->context); + _dbus_assert (bus_connection_is_active (connection)); return TRUE; diff --git a/bus/connection.h b/bus/connection.h index 4b9a754..7956e6a 100644 --- a/bus/connection.h +++ b/bus/connection.h @@ -43,6 +43,9 @@ void bus_connections_foreach (BusConnections void bus_connections_foreach_active (BusConnections *connections, BusConnectionForeachFunction function, void *data); +int bus_connections_get_n_incomplete (BusConnections *connections); +int bus_connections_get_n_completed (BusConnections *connections); + BusContext* bus_connections_get_context (BusConnections *connections); void bus_connections_increment_stamp (BusConnections *connections); BusContext* bus_connection_get_context (DBusConnection *connection); diff --git a/bus/main.c b/bus/main.c index 53038db..738a04f 100644 --- a/bus/main.c +++ b/bus/main.c @@ -75,7 +75,7 @@ signal_handler (int sig) static void usage (void) { - fprintf (stderr, DBUS_DAEMON_NAME " [--version] [--session] [--system] [--config-file=FILE] [--print-address[=DESCRIPTOR]] [--print-pid[=DESCRIPTOR]] [--fork] [--nofork] [--introspect] [--address=ADDRESS] [--systemd-activation]\n"); + fprintf (stderr, DBUS_DAEMON_NAME " [--version] [--session] [--system] [--config-file=FILE] [--print-address[=DESCRIPTOR]] [--print-pid[=DESCRIPTOR]] [--fork] [--nofork] [--introspect] [--address=ADDRESS] [--systemd-activation] [--exit-after-clients-exit]\n"); exit (1); } @@ -275,6 +275,7 @@ main (int argc, char **argv) dbus_bool_t is_session_bus; int force_fork; dbus_bool_t systemd_activation; + dbus_bool_t exit_after_clients_exit; if (!_dbus_string_init (&config_file)) return 1; @@ -293,6 +294,7 @@ main (int argc, char **argv) is_session_bus = FALSE; force_fork = FORK_FOLLOW_CONFIG_FILE; systemd_activation = FALSE; + exit_after_clients_exit = FALSE; prev_arg = NULL; i = 1; @@ -314,6 +316,8 @@ main (int argc, char **argv) force_fork = FORK_ALWAYS; else if (strcmp (arg, "--systemd-activation") == 0) systemd_activation = TRUE; + else if (strcmp (arg, "--exit-after-clients-exit") == 0) + exit_after_clients_exit = TRUE; else if (strcmp (arg, "--system") == 0) { check_two_config_files (&config_file, "system"); @@ -493,6 +497,7 @@ main (int argc, char **argv) &print_addr_pipe, &print_pid_pipe, _dbus_string_get_length(&address) > 0 ? &address : NULL, systemd_activation, + exit_after_clients_exit, &error); _dbus_string_free (&config_file); if (context == NULL) diff --git a/bus/test.c b/bus/test.c index 6efad6e..94fb2fa 100644 --- a/bus/test.c +++ b/bus/test.c @@ -323,7 +323,8 @@ bus_context_new_test (const DBusString *test_data_dir, } dbus_error_init (&error); - context = bus_context_new (&config_file, FALSE, NULL, NULL, NULL, FALSE, &error); + context = bus_context_new (&config_file, FALSE, NULL, NULL, NULL, + FALSE, FALSE, &error); if (context == NULL) { _DBUS_ASSERT_ERROR_IS_SET (&error); diff --git a/tools/dbus-launch.c b/tools/dbus-launch.c index 00815f9..37a9e4a 100644 --- a/tools/dbus-launch.c +++ b/tools/dbus-launch.c @@ -778,6 +778,9 @@ exec_dbus_daemon (dbus_bool_t use_path, argv[i++] = "--session"; } + if (autolaunch) + argv[i++] = "--exit-after-clients-exit"; + argv[i++] = NULL; if (use_path) @@ -946,56 +949,12 @@ main (int argc, char **argv) if (exit_with_session) verbose ("--exit-with-session enabled\n"); - if (autolaunch) - { -#ifndef DBUS_BUILD_X11 - fprintf (stderr, "Autolaunch requested, but X11 support not compiled in.\n" - "Cannot continue.\n"); - exit (1); -#else - char *address; - pid_t pid; - long wid; - - if (get_machine_uuid () == NULL) - { - fprintf (stderr, "Machine UUID not provided as arg to --autolaunch\n"); - exit (1); - } - - verbose ("Autolaunch enabled (using X11).\n"); - if (!exit_with_session) - { - verbose ("--exit-with-session automatically enabled\n"); - exit_with_session = TRUE; - } - - if (!x11_init ()) - { - fprintf (stderr, "Autolaunch error: X11 initialization failed.\n"); - exit (1); - } - - if (!x11_get_address (&address, &pid, &wid)) - { - fprintf (stderr, "Autolaunch error: X11 communication error.\n"); - exit (1); - } - - if (address != NULL) - { - verbose ("dbus-daemon is already running. Returning existing parameters.\n"); - pass_info (runprog, address, pid, wid, c_shell_syntax, - bourne_shell_syntax, binary_syntax, argc, argv, remaining_args); - exit (0); - } - } - else if (read_machine_uuid_if_needed()) - { - x11_init(); +#ifdef DBUS_BUILD_X11 + if (read_machine_uuid_if_needed()) + { + x11_init(); + } #endif - } - if (pipe (bus_pid_to_launcher_pipe) < 0 || pipe (bus_address_to_launcher_pipe) < 0 || @@ -1201,36 +1160,10 @@ main (int argc, char **argv) close (bus_pid_to_launcher_pipe[READ_END]); #ifdef DBUS_BUILD_X11 - if (xdisplay != NULL) + if (!autolaunch && xdisplay != NULL) { verbose("Saving x11 address\n"); ret2 = x11_save_address (bus_address, bus_pid, &wid); - /* Only get an existing dbus session when autolaunching */ - if (autolaunch) - { - if (ret2 == 0) - { - char *address = NULL; - /* another window got added. Return its address */ - bus_pid_to_kill = bus_pid; - if (x11_get_address (&address, &bus_pid, &wid) - && address != NULL) - { - verbose ("dbus-daemon is already running. Returning existing parameters.\n"); - /* Kill the old bus */ - kill_bus(); - pass_info (runprog, address, bus_pid, wid, - c_shell_syntax, bourne_shell_syntax, binary_syntax, - argc, argv, remaining_args); - } - } - if (ret2 < 0) - { - fprintf (stderr, "Error saving bus information.\n"); - bus_pid_to_kill = bus_pid; - kill_bus_and_exit (1); - } - } } #endif -- 1.7.3.3