Attachment 41262 Details for Bug 32509 – Scope autolaunch to requesting client, don't connect to X server

[patch] Scope autolaunch to requesting client, don't connect to X server

Scope-autolaunch-to-requesting-client-dont-connect.patch (text/plain), 15.96 KB, created by Colin Walters on 2010-12-19 12:03:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Walters

Created: 2010-12-19 12:03:03 UTC

Size: 15.96 KB

patch

obsolete

>From d00d4a6b41d65ff1058e5e152736d0ba24fce2f5 Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>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 &
><Control-d>
>
>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

Actions: View | Splinter Review

Attachments on bug 32509: 41260 | 41261 | 41262 | 41273

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.