After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 644851 - a11y bus should aways be started
a11y bus should aways be started
Status: RESOLVED FIXED
Product: at-spi
Classification: Platform
Component: at-spi2-core
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Li Yuan
Depends on:
Blocks:
 
 
Reported: 2011-03-15 18:17 UTC by Colin Walters
Modified: 2011-03-18 16:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bus: Rewrite a11y bus management, don't fall back to session bus (19.18 KB, patch)
2011-03-15 21:01 UTC, Colin Walters
none Details | Review
bus: Rewrite a11y bus management, don't fall back to session bus (21.55 KB, patch)
2011-03-15 22:11 UTC, Colin Walters
none Details | Review
bus: Rewrite a11y bus management, don't fall back to session bus (26.12 KB, patch)
2011-03-16 21:56 UTC, Colin Walters
needs-work Details | Review
bus: Rewrite a11y bus management, don't fall back to session bus (31.94 KB, patch)
2011-03-18 14:46 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2011-03-15 18:17:23 UTC
<walters> fcrozat: basically the a11y bus starting is predicated on the gsettings key
<walters> but it doesn't really work because if the key setting changes, all proccess are then using the wrong bus
 i'll file it
Comment 1 Colin Walters 2011-03-15 19:12:15 UTC
Actually, there are more problems here.

1) How the a11y bus address is accessed is via an X11 property.  But on startup, this is racy; something may try to access the property before we've finished launching the bus.
2) The a11y bus process is not lifecycle-chained to the session (either via X11 or the session bus)
Comment 2 Colin Walters 2011-03-15 19:22:12 UTC
3)  2>$HOME/.at-spi-dbus-bus.pid

Can't be relied on in NFS home scenarios, and in general, let's not rely on being able to write to $HOME.
Comment 3 Mike Gorse 2011-03-15 21:00:13 UTC
Currently, at-spi2-registryd is supposed to kill the dbus-daemon but fails to do so because I'm calling kill and passing in &pid rather than pid.  But this whole approach doesn't seem ideal because (a) it relies on a temporary file, (b) it is tied into gnome-session and thus only works on GNOME, and (c) it won't work if at-spi2-registryd was never started.  I'm thinking of writing a small program to monitor the session bus and kill the pid passed to it on a disconnect, and then having at-spi-dbus-bus invoke the program with the pid of the accessibility bus daemon, unless anyone reading this has a better suggestion (it feels somewhat heavy-handed, but I can't think of a better solution, since the accessibility bus should be tied to the session so that it knows which users should be allowed to connect).

As an aside, there was some discussion on dbus@lists.freedesktop.org a while ago about either creating a user bus or redefining the session bus to be more of a user bus[1].  If changes along these lines are made to the DBus specification and if setuid applications run by a user use the user's normal bus, then this seems like the right long-term solution to the issue, rather than launching a separate bus for a11y.

Anyway, I don't see any real harm in removing the GSETTINGS check, if people think that it should be done, although I don't think that it will in itself solve the issue of needing to restart the session for it to become accessible, since gail and atk-bridge are currently also loaded conditionally based on a GSettings key and afaik they are not automatically started if the key changes.

[1] http://lists.freedesktop.org/pipermail/dbus/2010-May/012777.html
Comment 4 Colin Walters 2011-03-15 21:01:45 UTC
Created attachment 183468 [details] [review]
bus: Rewrite a11y bus management, don't fall back to session bus

First of all, there should *always* be an a11y bus; if I enable
toolkit-accessibility, and apps pick that up but then libat-spi falls
back to the session bus, that's broken.

Fix this by rewriting the a11y bus launcher in C, making it a persistent
session service, and giving it a DBus API (bus name: org.a11y.Bus).  It
will start the bus process as a child dynamically; however if
toolkit-accessibility is enabled, we start it on startup.  For more
information, see the new file bus/README.
Comment 5 Colin Walters 2011-03-15 21:03:55 UTC
(In reply to comment #3)
> Currently, at-spi2-registryd is supposed to kill the dbus-daemon but fails to
> do so because I'm calling kill and passing in &pid rather than pid.

Oh, gross!  I'll remove that now, it's unnecessary with the attached patch.

>  But this
> whole approach doesn't seem ideal because (a) it relies on a temporary file,

Yes, always broken.

> (b) it is tied into gnome-session and thus only works on GNOME,

My patch makes this work too.

> and (c) it
> won't work if at-spi2-registryd was never started.  

...by installing a DBus .service file, so the session bus will start at-spi-bus-launcher when libatspi invokes the DBus call.
Comment 6 Colin Walters 2011-03-15 22:11:31 UTC
Created attachment 183471 [details] [review]
bus: Rewrite a11y bus management, don't fall back to session bus

First of all, there should *always* be an a11y bus; if I enable
toolkit-accessibility, and apps pick that up but then libat-spi falls
back to the session bus, that's broken.

Fix this by rewriting the a11y bus launcher in C, making it a persistent
session service, and giving it a DBus API (bus name: org.a11y.Bus).  It
will start the bus process as a child dynamically; however if
toolkit-accessibility is enabled, we start it on startup.  For more
information, see the new file bus/README.

Details:

* Create a .service file so the session bus autostarts us if
  necessary
* The .desktop file is changed to use --start-immediately
* Remove the kill_accessibility_bus() hack in registryd; instead
  we chain at-spi-bus-launcher to the session bus lifecycle.
* Change at-spi bus lookup to try both the new session bus API
  and the X11 root window property.
Comment 7 Colin Walters 2011-03-15 22:13:46 UTC
Updated patch:

* handles SIGTERM; basically you can:
$ gsettings set org.gnome.desktop.toolkit-accessibility false
and gnome-session sends us SIGTERM which we then handle by killing our child process.

* updated both places to use org.a11y.Bus for locating

Only lightly tested.  I admit to not fully understanding all the components here, so comments/testing appreciated.
Comment 8 Frederic Crozat 2011-03-16 09:53:33 UTC
I've tested the patch : every application is complaining
AT-SPI: couldn't connect to bus: In D-Bus address, character '
' should have been escaped 

and a11y doesn't work
Comment 9 Colin Walters 2011-03-16 21:56:45 UTC
Created attachment 183576 [details] [review]
bus: Rewrite a11y bus management, don't fall back to session bus

* Also check the GConf key like the old shell script did
* Use the same method to find the bus in registryd
Comment 10 Frederic Crozat 2011-03-17 13:05:39 UTC
still problematic when enabling a11y, the entire desktop isn't responding now.
Comment 11 Mike Gorse 2011-03-18 13:33:20 UTC
Comment on attachment 183576 [details] [review]
bus: Rewrite a11y bus management, don't fall back to session bus

The patch looks fine over-all.  A few minor things:

- I was getting the same error as Frederic; we're getting a trailing newline
  from dbus-daemon that we need to strip when setting app->a11y_bus_address.
  Adding in this line fixes that for me:

  app->a11y_bus_address [strcspn (app->a11y_bus_address, "\r\n")] = '\0';

- bus/Makefile.am needs to do something like setting servicedir and
service_DATA (as in registry/Makefile.am) to install the service file.

- I tested with gnomesu (which OpenSUSE 11.3 uses for YaST at least; not sure about 11.4), and at-spi-bus-launcher got launched again as root with a different DBUS_SESSION_BUS_ADDRESS value (I'm not sure where this was being set, since it looks as though it was unset initially), so the root application was not accessible, which is the same problem that I'd have when simply using the session bus.  Checking for a bus address via X11 before org.a11y.Bus instead of the other way around gets things to work for me.

>From def1749d9b77825c70802fee352a54fff4f6e8d0 Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>Date: Tue, 15 Mar 2011 17:00:35 -0400
>Subject: [PATCH] bus: Rewrite a11y bus management, don't fall back to session bus
>
>First of all, there should *always* be an a11y bus; if I enable
>toolkit-accessibility, and apps pick that up but then libat-spi falls
>back to the session bus, that's broken.
>
>Fix this by rewriting the a11y bus launcher in C, making it a persistent
>session service, and giving it a DBus API (bus name: org.a11y.Bus).  It
>will start the bus process as a child dynamically; however if
>toolkit-accessibility is enabled, we start it on startup.  For more
>information, see the new file bus/README.
>
>Details:
>
>* Create a .service file so the session bus autostarts us if
>  necessary
>* The .desktop file is changed to use --start-immediately
>* Remove the kill_accessibility_bus() hack in registryd; instead
>  we chain at-spi-bus-launcher to the session bus lifecycle.
>* Change at-spi bus lookup to try both the new session bus API
>  and the X11 root window property.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=644851
>---
> atspi/atspi-misc.c             |  119 ++++++++----
> bus/Makefile.am                |   27 ++-
> bus/README                     |   10 +
> bus/accessibility.conf         |    2 -
> bus/at-spi-bus-launcher.c      |  392 ++++++++++++++++++++++++++++++++++++++++
> bus/at-spi-dbus-bus.desktop.in |    2 +-
> bus/at-spi-dbus-bus.in         |   13 --
> bus/org.a11y.Bus.service.in    |    3 +
> configure.ac                   |    5 +-
> registryd/registry-main.c      |  176 ++++++++++--------
> 10 files changed, 605 insertions(+), 144 deletions(-)
> create mode 100644 bus/README
> create mode 100644 bus/at-spi-bus-launcher.c
> delete mode 100644 bus/at-spi-dbus-bus.in
> create mode 100644 bus/org.a11y.Bus.service.in
>
>diff --git a/atspi/atspi-misc.c b/atspi/atspi-misc.c
>index 2adea05..db01097 100644
>--- a/atspi/atspi-misc.c
>+++ b/atspi/atspi-misc.c
>@@ -805,9 +805,8 @@ spi_display_name (void)
>   return canonical_display_name;
> }
> 
>-/* TODO: Avoid having duplicate functions for this here and in at-spi2-atk */
>-static DBusConnection *
>-get_accessibility_bus ()
>+static char *
>+get_accessibility_bus_address_x11 (void)
> {
>   Atom AT_SPI_BUS;
>   Atom actual_type;
>@@ -817,56 +816,99 @@ get_accessibility_bus ()
>   unsigned long nitems;
>   unsigned long leftover;
> 
>-  DBusConnection *bus = NULL;
>-  DBusError error;
>-
>   bridge_display = XOpenDisplay (spi_display_name ());
>   if (!bridge_display)
>     {
>       g_warning (_("AT-SPI: Could not get the display\n"));
>       return NULL;
>     }
>-
>+      
>   AT_SPI_BUS = XInternAtom (bridge_display, "AT_SPI_BUS", False);
>   XGetWindowProperty (bridge_display,
>-                      XDefaultRootWindow (bridge_display),
>-                      AT_SPI_BUS, 0L,
>-                      (long) BUFSIZ, False,
>-                      (Atom) 31, &actual_type, &actual_format,
>-                      &nitems, &leftover, &data);
>+		      XDefaultRootWindow (bridge_display),
>+		      AT_SPI_BUS, 0L,
>+		      (long) BUFSIZ, False,
>+		      (Atom) 31, &actual_type, &actual_format,
>+		      &nitems, &leftover, &data);
>   XCloseDisplay (bridge_display);
> 
>-  dbus_error_init (&error);
>+  return g_strdup (data);
>+}
>+
>+static char *
>+get_accessibility_bus_address_dbus (void)
>+{
>+  DBusConnection *session_bus = NULL;
>+  DBusMessage *message;
>+  DBusMessage *reply;
>+  char *address = NULL;
> 
>-  if (data == NULL)
>+  session_bus = dbus_bus_get (DBUS_BUS_SESSION, NULL);
>+  if (!session_bus)
>+    return NULL;
>+
>+  message = dbus_message_new_method_call ("org.a11y.Bus",
>+					  "/org/a11y/bus",
>+					  "org.a11y.Bus",
>+					  "GetAddress");
>+
>+  reply = dbus_connection_send_with_reply_and_block (session_bus,
>+						     message,
>+						     -1,
>+						     NULL);
>+  dbus_message_unref (message);
>+
>+  if (!reply)
>+    return NULL;
>+  
>+  {
>+    const char *tmp_address;
>+    if (!dbus_message_get_args (reply,
>+				NULL,
>+				DBUS_TYPE_STRING,
>+				&tmp_address,
>+				DBUS_TYPE_INVALID))
>+      {
>+	dbus_message_unref (reply);
>+	return NULL;
>+      }
>+    address = g_strdup (tmp_address);
>+    dbus_message_unref (reply);
>+  }
>+  
>+  return address;
>+}
>+
>+/* TODO: Avoid having duplicate functions for this here and in at-spi2-atk */
>+static DBusConnection *
>+get_accessibility_bus (void)
>+{
>+  DBusConnection *bus = NULL;
>+  DBusError error;
>+  char *address;
>+
>+  address = get_accessibility_bus_address_dbus ();
>+  if (!address)
>+    address = get_accessibility_bus_address_x11 ();
>+  if (!address)
>+    return NULL;
>+
>+  dbus_error_init (&error);
>+  bus = dbus_connection_open (address, &error);
>+  if (!bus)
>     {
>-      g_warning
>-        (_("AT-SPI: Accessibility bus not found - Using session bus.\n"));
>-      bus = dbus_bus_get (DBUS_BUS_SESSION, &error);
>-      if (!bus)
>-        {
>-          g_warning (_("AT-SPI: Couldn't connect to bus: %s\n"), error.message);
>-          return NULL;
>-        }
>+      g_warning (_("AT-SPI: Couldn't connect to accessibility bus: %s\n"), error.message);
>+      return NULL;
>     }
>   else
>     {
>-      bus = dbus_connection_open (data, &error);
>-      if (!bus)
>-        {
>-          g_warning (_("AT-SPI: Couldn't connect to bus: %s\n"), error.message);
>-          return NULL;
>-        }
>-      else
>-        {
>-          if (!dbus_bus_register (bus, &error))
>-            {
>-              g_warning (_("AT-SPI: Couldn't register with bus: %s\n"), error.message);
>-              return NULL;
>-            }
>-        }
>+      if (!dbus_bus_register (bus, &error))
>+	{
>+	  g_warning (_("AT-SPI: Couldn't register with accessibility bus: %s\n"), error.message);
>+	  return NULL;
>+	}
>     }
>-
>+  
>   return bus;
> }
> 
>@@ -897,10 +939,7 @@ atspi_init (void)
>   dbus_error_init (&error);
>   bus = get_accessibility_bus ();
>   if (!bus)
>-  {
>-    g_warning ("Couldn't get session bus");
>     return 2;
>-  }
>   dbus_bus_register (bus, &error);
>   dbus_connection_setup_with_g_main(bus, g_main_context_default());
>   dbus_connection_add_filter (bus, atspi_dbus_filter, NULL, NULL);
>diff --git a/bus/Makefile.am b/bus/Makefile.am
>index 1f5a7d1..2151f1b 100644
>--- a/bus/Makefile.am
>+++ b/bus/Makefile.am
>@@ -1,20 +1,27 @@
>+EXTRA_DIST=accessibility.conf
>+CLEANFILES=
>+
> busconfigdir = $(sysconfdir)/at-spi2
> busconfig_DATA = accessibility.conf
> 
>-atspidbusdir = $(libexecdir)
>-atspidbus_SCRIPTS = at-spi-dbus-bus
>+libexec_PROGRAMS = at-spi-bus-launcher
>+at_spi_bus_launcher_SOURCES = at-spi-bus-launcher.c
>+at_spi_bus_launcher_CPPFLAGS = -DSYSCONFDIR=\"$(sysconfdir)\" \
>+                               -DDBUS_DAEMON=\"$(DBUS_DAEMON)\"
>+at_spi_bus_launcher_CFLAGS = $(GIO_CFLAGS)
>+at_spi_bus_launcher_LDADD = $(GIO_LIBS) $(X_LIBS)
> 
> default_sessiondir = $(sysconfdir)/xdg/autostart
> default_session_DATA = at-spi-dbus-bus.desktop
> 
>+substitutions = "s,@libexecdir[@],$(libexecdir),"
> at-spi-dbus-bus.desktop: at-spi-dbus-bus.desktop.in
>-	sed -e "s,@libexecdir[@],$(libexecdir)," $< > $@.tmp && mv $@.tmp $@
>+	sed -e$ $(substitutions)  $< > $@.tmp && mv $@.tmp $@
>+EXTRA_DIST += at-spi-dbus-bus.desktop.in
>+CLEANFILES += at-spi-dbus-bus.desktop
> 
>-EXTRA_DIST= \
>-	accessibility.conf \
>-	at-spi-dbus-bus.in \
>-	at-spi-dbus-bus.desktop.in
>+org.a11y.Bus.service: org.a11y.Bus.service.in
>+	sed -e $(substitutions) $< > $@.tmp && mv $@.tmp $@
>+EXTRA_DIST += org.a11y.Bus.service.in
>+CLEANFILES += org.a11y.Bus.service
> 
>-CLEANFILES= \
>-	at-spi-dbus-bus.desktop \
>-	at-spi-dbus-bus
>diff --git a/bus/README b/bus/README
>new file mode 100644
>index 0000000..40b9ad6
>--- /dev/null
>+++ b/bus/README
>@@ -0,0 +1,10 @@
>+The a11y bus is accessed via two mechanisms:
>+
>+1) The DBus session bus, service "org.a11y.Bus", method "GetAddress")
>+2) The X11 root window property AT_SPI_BUS
>+
>+If the "toolkit-accessibility" variable is set, the bus is launched
>+immediately (and will be accessible immediately via the X11 property).
>+Otherwise, it will be spawned dynamically.
>+
>+
>diff --git a/bus/accessibility.conf b/bus/accessibility.conf
>index d9703e0..b9367d0 100644
>--- a/bus/accessibility.conf
>+++ b/bus/accessibility.conf
>@@ -3,8 +3,6 @@
> 
>   <type>accessibility</type>
> 
>-  <fork/>
>-
>   <standard_session_servicedirs/>
> 
>   <auth>EXTERNAL</auth>
>diff --git a/bus/at-spi-bus-launcher.c b/bus/at-spi-bus-launcher.c
>new file mode 100644
>index 0000000..26172e0
>--- /dev/null
>+++ b/bus/at-spi-bus-launcher.c
>@@ -0,0 +1,392 @@
>+/* -*- mode: c; c-basic-offset: 2; indent-tabs-mode: nil; -*-
>+ * 
>+ * at-spi-bus-launcher: Manage the a11y bus as a child process 
>+ *
>+ * Copyright 2011 Red Hat, Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Library General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Library General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Library General Public
>+ * License along with this library; if not, write to the
>+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>+ * Boston, MA 02111-1307, USA.
>+ */
>+
>+#include "config.h"
>+
>+#include <unistd.h>
>+#include <string.h>
>+#include <signal.h>
>+#include <sys/wait.h>
>+#include <errno.h>
>+
>+#include <gio/gio.h>
>+#include <X11/Xlib.h>
>+#include <X11/Xatom.h>
>+
>+typedef enum {
>+  A11Y_BUS_STATE_IDLE = 0,
>+  A11Y_BUS_STATE_READING_ADDRESS,
>+  A11Y_BUS_STATE_RUNNING,
>+  A11Y_BUS_STATE_ERROR
>+} A11yBusState;
>+
>+typedef struct {
>+  GMainLoop *loop;
>+  gboolean launch_immediately;
>+  GDBusConnection *session_bus;
>+
>+  A11yBusState state;
>+  /* -1 == error, 0 == pending, > 0 == running */
>+  int a11y_bus_pid;
>+  char *a11y_bus_address;
>+  int pipefd[2];
>+  char *a11y_launch_error_message;
>+} A11yBusLauncher;
>+
>+static A11yBusLauncher *_global_app = NULL;
>+
>+static const gchar introspection_xml[] =
>+  "<node>"
>+  "  <interface name='org.a11y.Bus'>"
>+  "    <method name='GetAddress'>"
>+  "      <arg type='s' name='address' direction='out'/>"
>+  "    </method>"
>+  "  </interface>"
>+  "</node>";
>+static GDBusNodeInfo *introspection_data = NULL;
>+
>+static void
>+setup_bus_child (gpointer data)
>+{
>+  A11yBusLauncher *app = data;
>+  (void) app;
>+
>+  close (app->pipefd[0]);
>+  dup2 (app->pipefd[1], 3);
>+  close (app->pipefd[1]);
>+
>+  /* On Linux, tell the bus process to exit if this process goes away */
>+#ifdef __linux
>+#include <sys/prctl.h>
>+  prctl (PR_SET_PDEATHSIG, 15);
>+#endif  
>+}
>+
>+/**
>+ * unix_read_all_fd_to_string:
>+ *
>+ * Read all data from a file descriptor to a C string buffer.
>+ */
>+static gboolean
>+unix_read_all_fd_to_string (int      fd,
>+                            char    *buf,
>+                            ssize_t  max_bytes)
>+{
>+  ssize_t bytes_read;
>+
>+  while (max_bytes > 1 && (bytes_read = read (fd, buf, MAX (4096, max_bytes - 1))))
>+    {
>+      if (bytes_read < 0)
>+        return FALSE;
>+      buf += bytes_read;
>+      max_bytes -= bytes_read;
>+    }
>+  *buf = '\0';
>+  return TRUE;
>+}
>+
>+static void
>+on_bus_exited (GPid     pid,
>+               gint     status,
>+               gpointer data)
>+{
>+  A11yBusLauncher *app = data;
>+  
>+  app->a11y_bus_pid = -1;
>+  app->state = A11Y_BUS_STATE_ERROR;
>+  if (app->a11y_launch_error_message == NULL)
>+    {
>+      if (WIFEXITED (status))
>+        app->a11y_launch_error_message = g_strdup_printf ("Bus exited with code %d", WEXITSTATUS (status));
>+      else if (WIFSIGNALED (status))
>+        app->a11y_launch_error_message = g_strdup_printf ("Bus killed by signal %d", WTERMSIG (status));
>+      else if (WIFSTOPPED (status))
>+        app->a11y_launch_error_message = g_strdup_printf ("Bus stopped by signal %d", WSTOPSIG (status));
>+    }
>+  g_main_loop_quit (app->loop);
>+} 
>+
>+static void
>+ensure_a11y_bus (A11yBusLauncher *app)
>+{
>+  GPid pid;
>+  char *argv[] = { DBUS_DAEMON, NULL, "--nofork", "--print-address", "3", NULL };
>+  char addr_buf[2048];
>+  GError *error = NULL;
>+
>+  if (app->a11y_bus_pid != 0)
>+    return;
>+  
>+  argv[1] = g_strdup_printf ("--config-file=%s/at-spi2/accessibility.conf", SYSCONFDIR);
>+
>+  if (pipe (app->pipefd) < 0)
>+    g_error ("Failed to create pipe: %s", strerror (errno));
>+  
>+  if (!g_spawn_async (NULL,
>+                      argv,
>+                      NULL,
>+                      G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD,
>+                      setup_bus_child,
>+                      app,
>+                      &pid,
>+                      &error))
>+    {
>+      app->a11y_bus_pid = -1;
>+      app->a11y_launch_error_message = g_strdup (error->message);
>+      g_clear_error (&error);
>+      goto error;
>+    }
>+
>+  close (app->pipefd[1]);
>+  app->pipefd[1] = -1;
>+
>+  g_child_watch_add (pid, on_bus_exited, app);
>+
>+  app->state = A11Y_BUS_STATE_READING_ADDRESS;
>+  app->a11y_bus_pid = pid;
>+  g_debug ("Launched a11y bus, child is %ld", (long) pid);
>+  if (!unix_read_all_fd_to_string (app->pipefd[0], addr_buf, sizeof (addr_buf)))
>+    {
>+      app->a11y_launch_error_message = g_strdup_printf ("Failed to read address: %s", strerror (errno));
>+      kill (app->a11y_bus_pid, SIGTERM);
>+      goto error;
>+    }
>+  close (app->pipefd[0]);
>+  app->pipefd[0] = -1;
>+  app->state = A11Y_BUS_STATE_RUNNING;
>+  app->a11y_bus_address = g_strdup (addr_buf);
>+
>+  {
>+    Display *display = XOpenDisplay (NULL);
>+    if (display)
>+      {
>+        Atom bus_address_atom = XInternAtom (display, "AT_SPI_BUS", False);
>+        XChangeProperty (display,
>+                         XDefaultRootWindow (display),
>+                         bus_address_atom,
>+                         XA_STRING, 8, PropModeReplace,
>+                         (guchar *) app->a11y_bus_address, strlen (app->a11y_bus_address));
>+      }
>+    XFlush (display);
>+    XCloseDisplay (display);
>+  }
>+
>+  return;
>+  
>+ error:
>+  close (app->pipefd[0]);
>+  close (app->pipefd[1]);
>+  app->state = A11Y_BUS_STATE_ERROR;
>+}
>+
>+static void
>+handle_method_call (GDBusConnection       *connection,
>+                    const gchar           *sender,
>+                    const gchar           *object_path,
>+                    const gchar           *interface_name,
>+                    const gchar           *method_name,
>+                    GVariant              *parameters,
>+                    GDBusMethodInvocation *invocation,
>+                    gpointer               user_data)
>+{
>+  A11yBusLauncher *app = user_data;
>+
>+  if (g_strcmp0 (method_name, "GetAddress") == 0)
>+    {
>+      ensure_a11y_bus (app);
>+      if (app->a11y_bus_pid > 0)
>+        g_dbus_method_invocation_return_value (invocation,
>+                                               g_variant_new ("(s)", app->a11y_bus_address));
>+      else
>+        g_dbus_method_invocation_return_dbus_error (invocation,
>+                                                    "org.a11y.Bus.Error",
>+                                                    app->a11y_launch_error_message);
>+    }
>+}
>+
>+static const GDBusInterfaceVTable interface_vtable =
>+{
>+  handle_method_call,
>+  NULL,
>+  NULL  /* handle_set_property */
>+};
>+
>+static void
>+on_bus_acquired (GDBusConnection *connection,
>+                 const gchar     *name,
>+                 gpointer         user_data)
>+{
>+  A11yBusLauncher *app = user_data;
>+  GError *error;
>+  guint registration_id;
>+  
>+  if (connection == NULL)
>+    {
>+      g_main_loop_quit (app->loop);
>+      return;
>+    }
>+  app->session_bus = connection;
>+
>+  if (app->launch_immediately)
>+    {
>+      ensure_a11y_bus (app);
>+      if (app->state == A11Y_BUS_STATE_ERROR)
>+        {
>+          g_main_loop_quit (app->loop);
>+          return;
>+        }
>+    }
>+
>+  error = NULL;
>+  registration_id = g_dbus_connection_register_object (connection,
>+                                                       "/org/a11y/bus",
>+                                                       introspection_data->interfaces[0],
>+                                                       &interface_vtable,
>+                                                       _global_app,
>+                                                       NULL,
>+                                                       &error);
>+  if (registration_id == 0)
>+    g_error ("%s", error->message);
>+}
>+
>+static void
>+on_name_lost (GDBusConnection *connection,
>+              const gchar     *name,
>+              gpointer         user_data)
>+{
>+  A11yBusLauncher *app = user_data;
>+  if (app->session_bus == NULL
>+      && connection == NULL
>+      && app->a11y_launch_error_message == NULL)
>+    app->a11y_launch_error_message = g_strdup ("Failed to connect to session bus");
>+  g_main_loop_quit (app->loop);
>+}
>+
>+static void
>+on_name_acquired (GDBusConnection *connection,
>+                  const gchar     *name,
>+                  gpointer         user_data)
>+{
>+  A11yBusLauncher *app = user_data;
>+  (void) app;
>+}
>+
>+static int sigterm_pipefd[2];
>+
>+static void
>+sigterm_handler (int signum)
>+{
>+  write (sigterm_pipefd[1], "X", 1);
>+}
>+
>+static gboolean
>+on_sigterm_pipe (GIOChannel  *channel,
>+                 GIOCondition condition,
>+                 gpointer     data)
>+{
>+  A11yBusLauncher *app = data;
>+  
>+  g_main_loop_quit (app->loop);
>+
>+  return FALSE;
>+}
>+
>+static void
>+init_sigterm_handling (A11yBusLauncher *app)
>+{
>+  GIOChannel *sigterm_channel;
>+
>+  if (pipe (sigterm_pipefd) < 0)
>+    g_error ("Failed to create pipe: %s", strerror (errno));
>+  signal (SIGTERM, sigterm_handler);
>+
>+  sigterm_channel = g_io_channel_unix_new (sigterm_pipefd[0]);
>+  g_io_add_watch (sigterm_channel,
>+                  G_IO_IN | G_IO_ERR | G_IO_HUP,
>+                  on_sigterm_pipe,
>+                  app);
>+}
>+
>+static gboolean
>+is_a11y_using_corba (void)
>+{
>+  char *gconf_argv[] = { "gconftool-2", "--get", "/desktop/gnome/interface/at-spi-corba", NULL };
>+  char *stdout = NULL;
>+  int estatus;
>+  gboolean result = FALSE;
>+
>+  if (!g_spawn_sync (NULL, gconf_argv, NULL,
>+                     G_SPAWN_SEARCH_PATH, NULL, NULL, &stdout, NULL, &estatus, NULL))
>+    goto out;
>+  if (estatus != 0)
>+    goto out;
>+  if (g_str_has_prefix (stdout, "true"))
>+    result = TRUE;
>+ out:
>+  g_free (stdout);
>+  return result;
>+}
>+
>+int
>+main (int    argc,
>+      char **argv)
>+{
>+  GError *error = NULL;
>+  GMainLoop *loop;
>+  GDBusConnection *session_bus;
>+  int name_owner_id;
>+
>+  g_type_init ();
>+
>+  if (is_a11y_using_corba ())
>+    return 0;
>+
>+  _global_app = g_slice_new0 (A11yBusLauncher);
>+  _global_app->loop = g_main_loop_new (NULL, FALSE);
>+  _global_app->launch_immediately = (argc == 2 && strcmp (argv[1], "--launch-immediately") == 0);
>+
>+  init_sigterm_handling (_global_app);
>+
>+  introspection_data = g_dbus_node_info_new_for_xml (introspection_xml, NULL);
>+  g_assert (introspection_data != NULL);
>+
>+  name_owner_id = g_bus_own_name (G_BUS_TYPE_SESSION,
>+                                  "org.a11y.Bus",
>+                                  G_BUS_NAME_OWNER_FLAGS_ALLOW_REPLACEMENT,
>+                                  on_bus_acquired,
>+                                  on_name_acquired,
>+                                  on_name_lost,
>+                                  _global_app,
>+                                  NULL);
>+
>+  g_main_loop_run (_global_app->loop);
>+
>+  if (_global_app->a11y_bus_pid > 0)
>+    kill (_global_app->a11y_bus_pid, SIGTERM);
>+    
>+  if (_global_app->a11y_launch_error_message)
>+    {
>+      g_printerr ("Failed to launch bus: %s", _global_app->a11y_launch_error_message);
>+      return 1;
>+    }
>+  return 0;
>+}
>diff --git a/bus/at-spi-dbus-bus.desktop.in b/bus/at-spi-dbus-bus.desktop.in
>index 8290478..0d25fda 100644
>--- a/bus/at-spi-dbus-bus.desktop.in
>+++ b/bus/at-spi-dbus-bus.desktop.in
>@@ -1,7 +1,7 @@
> [Desktop Entry]
> Type=Application
> Name=AT SPI D-Bus Bus
>-Exec=@libexecdir@/at-spi-dbus-bus
>+Exec=@libexecdir@/at-spi-bus-launcher --launch-immediately
> OnlyShowIn=GNOME;
> NoDisplay=true
> AutostartCondition=GSETTINGS org.gnome.desktop.interface toolkit-accessibility
>diff --git a/bus/at-spi-dbus-bus.in b/bus/at-spi-dbus-bus.in
>deleted file mode 100644
>index af92d25..0000000
>--- a/bus/at-spi-dbus-bus.in
>+++ /dev/null
>@@ -1,13 +0,0 @@
>-#!/bin/sh
>-
>-prefix="@prefix@"
>-sysconfdir="@sysconfdir@"
>-dbusdaemon="@DBUS_DAEMON@"
>-
>-var=`gconftool-2 --get /desktop/gnome/interface/at-spi-corba`
>-if [ "$var" == "true" ]; then
>-  exit 0
>-fi
>-
>-address=`${dbusdaemon} --config-file=${sysconfdir}/at-spi2/accessibility.conf --print-address 1 --print-pid 2 2>$HOME/.at-spi-dbus-bus.pid`
>-xprop -root -f AT_SPI_BUS 8s -set AT_SPI_BUS ${address}
>diff --git a/bus/org.a11y.Bus.service.in b/bus/org.a11y.Bus.service.in
>new file mode 100644
>index 0000000..60edc28
>--- /dev/null
>+++ b/bus/org.a11y.Bus.service.in
>@@ -0,0 +1,3 @@
>+[D-BUS Service]
>+Name=org.a11y.Bus
>+Exec=@libexecdir@/at-spi-bus-launcher
>diff --git a/configure.ac b/configure.ac
>index f5a581e..aee668c 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -55,6 +55,10 @@ PKG_CHECK_MODULES(GOBJ, [gobject-2.0 >= 2.0.0])
> AC_SUBST(GOBJ_LIBS)
> AC_SUBST(GOBJ_CFLAGS)
> 
>+PKG_CHECK_MODULES(GIO, [gio-2.0 >= 2.28])
>+AC_SUBST(GIO_LIBS)
>+AC_SUBST(GIO_CFLAGS)
>+
> # --------------------------------------------------------------------
> # Find DL functionality
> 
>@@ -192,7 +196,6 @@ AC_CONFIG_FILES([Makefile po/Makefile.in
> dbind/dbind-config.h
> 	atspi/Makefile
> 	registryd/Makefile
>-	bus/at-spi-dbus-bus
> 	bus/Makefile
> doc/Makefile
> doc/libatspi/Makefile
>diff --git a/registryd/registry-main.c b/registryd/registry-main.c
>index 083de19..e07cba3 100644
>--- a/registryd/registry-main.c
>+++ b/registryd/registry-main.c
>@@ -88,36 +88,8 @@ session_manager_connect (void)
> }
> 
> static void
>-kill_accessibility_bus ()
>-{
>-  FILE *fp;
>-  const char *home;
>-  char *name;
>-  int pid;
>-
>-  home = getenv ("HOME");
>-  if (!home)
>-    return;
>-  name = g_strconcat (home, "/", ".atspi-dbus-bus.pid", NULL);
>-  if (!name)
>-    return;
>-
>-  fp = fopen (name, "r");
>-  if (fp)
>-  {
>-    if (fscanf (fp, "%d", &pid) == 1)
>-    {
>-      kill (&pid, SIGTERM);
>-    }
>-    fclose (fp);
>-  }
>-  g_free (name);
>-}
>-
>-static void
> stop_cb (gpointer data)
> {
>-        kill_accessibility_bus ();
>         g_main_loop_quit (mainloop);
> }
> 
>@@ -151,7 +123,6 @@ query_end_session_cb (guint flags, gpointer data)
> static void
> end_session_cb (guint flags, gpointer data)
> {
>-        kill_accessibility_bus ();
>         end_session_response (TRUE, NULL);
>         g_main_loop_quit (mainloop);
> }
>@@ -250,59 +221,110 @@ spi_display_name (void)
>  * may be employed in the future for accessing the registry daemon
>  * bus name.
>  */
>+static char *
>+get_accessibility_bus_address_x11 (void)
>+{
>+  Atom AT_SPI_BUS;
>+  Atom actual_type;
>+  Display *bridge_display;
>+  int actual_format;
>+  unsigned char *data = NULL;
>+  unsigned long nitems;
>+  unsigned long leftover;
>+
>+  bridge_display = XOpenDisplay (spi_display_name ());
>+  if (!bridge_display)
>+    {
>+      g_warning ("Could not open X display");
>+      return NULL;
>+    }
>+      
>+  AT_SPI_BUS = XInternAtom (bridge_display, "AT_SPI_BUS", False);
>+  XGetWindowProperty (bridge_display,
>+		      XDefaultRootWindow (bridge_display),
>+		      AT_SPI_BUS, 0L,
>+		      (long) BUFSIZ, False,
>+		      (Atom) 31, &actual_type, &actual_format,
>+		      &nitems, &leftover, &data);
>+  XCloseDisplay (bridge_display);
>+
>+  return g_strdup (data);
>+}
>+
>+static char *
>+get_accessibility_bus_address_dbus (void)
>+{
>+  DBusConnection *session_bus = NULL;
>+  DBusMessage *message;
>+  DBusMessage *reply;
>+  char *address = NULL;
>+
>+  session_bus = dbus_bus_get (DBUS_BUS_SESSION, NULL);
>+  if (!session_bus)
>+    return NULL;
>+
>+  message = dbus_message_new_method_call ("org.a11y.Bus",
>+					  "/org/a11y/bus",
>+					  "org.a11y.Bus",
>+					  "GetAddress");
>+
>+  reply = dbus_connection_send_with_reply_and_block (session_bus,
>+						     message,
>+						     -1,
>+						     NULL);
>+  dbus_message_unref (message);
>+
>+  if (!reply)
>+    return NULL;
>+  
>+  {
>+    const char *tmp_address;
>+    if (!dbus_message_get_args (reply,
>+				NULL,
>+				DBUS_TYPE_STRING,
>+				&tmp_address,
>+				DBUS_TYPE_INVALID))
>+      {
>+	dbus_message_unref (reply);
>+	return NULL;
>+      }
>+    address = g_strdup (tmp_address);
>+    dbus_message_unref (reply);
>+  }
>+  
>+  return address;
>+}
> 
> static DBusConnection *
> spi_get_bus (void)
> {
>-     Atom AT_SPI_BUS;
>-     Atom actual_type;
>-     Display *bridge_display;
>-     int actual_format;
>-     unsigned char *data = NULL;  
>-     unsigned long nitems;
>-     unsigned long leftover;
>-
>-     DBusConnection *bus = NULL;
>-     DBusError       error;
>-
>-     bridge_display = XOpenDisplay (spi_display_name ());
>-     if (!bridge_display)
>-	g_error ("AT_SPI: Could not get the display");
>-
>-     AT_SPI_BUS = XInternAtom (bridge_display, "AT_SPI_BUS", FALSE); 
>-     XGetWindowProperty(bridge_display, 
>-                        XDefaultRootWindow (bridge_display),
>-                        AT_SPI_BUS, 0L,
>-                        (long)BUFSIZ, False,
>-                        (Atom) 31, &actual_type, &actual_format,
>-                        &nitems, &leftover, &data);
>-
>-     dbus_error_init (&error);
>-
>-     if (data == NULL)
>-     {
>-         g_warning ("AT-SPI: Accessibility bus bus not found - Using session bus.\n");
>-         bus = dbus_bus_get (DBUS_BUS_SESSION, &error);
>-         if (!bus)
>-             g_error ("AT-SPI: Couldn't connect to bus: %s\n", error.message);
>-     }
>-     else
>-     {
>-	 bus = dbus_connection_open (data, &error);
>-	 XFree (data);
>-         if (!bus)
>-         {
>-             g_error ("AT-SPI: Couldn't connect to bus: %s\n", error.message);
>-         }
>-	 else
>-         {
>-	     if (!dbus_bus_register (bus, &error))
>-	         g_error ("AT-SPI: Couldn't register with bus: %s\n", error.message);
>-         } 
>-     }
>+  DBusConnection *bus = NULL;
>+  DBusError error;
>+  char *address;
> 
>-  XCloseDisplay (bridge_display);
>-     return bus;
>+  address = get_accessibility_bus_address_dbus ();
>+  if (!address)
>+    address = get_accessibility_bus_address_x11 ();
>+  if (!address)
>+    return NULL;
>+
>+  dbus_error_init (&error);
>+  bus = dbus_connection_open (address, &error);
>+  if (!bus)
>+    {
>+      g_warning ("Couldn't connect to accessibility bus: %s", error.message);
>+      return NULL;
>+    }
>+  else
>+    {
>+      if (!dbus_bus_register (bus, &error))
>+	{
>+	  g_warning ("Couldn't register with accessibility bus: %s", error.message);
>+	  return NULL;
>+	}
>+    }
>+  
>+  return bus;
> }
> 
> /*---------------------------------------------------------------------------*/
>-- 
>1.7.4.1
Comment 12 Colin Walters 2011-03-18 13:44:30 UTC
(In reply to comment #11)
> (From update of attachment 183576 [details] [review])
> The patch looks fine over-all.  A few minor things:
> 
> - I was getting the same error as Frederic; we're getting a trailing newline
>   from dbus-daemon that we need to strip when setting app->a11y_bus_address.
>   Adding in this line fixes that for me:
> 
>   app->a11y_bus_address [strcspn (app->a11y_bus_address, "\r\n")] = '\0';

Looks right, thanks!

> - bus/Makefile.am needs to do something like setting servicedir and
> service_DATA (as in registry/Makefile.am) to install the service file.

Will fix.

> - I tested with gnomesu (which OpenSUSE 11.3 uses for YaST at least; not sure
> about 11.4), and at-spi-bus-launcher got launched again as root with a
> different DBUS_SESSION_BUS_ADDRESS value (I'm not sure where this was being
> set, since it looks as though it was unset initially), so the root application
> was not accessible, which is the same problem that I'd have when simply using
> the session bus.  Checking for a bus address via X11 before org.a11y.Bus
> instead of the other way around gets things to work for me.

Hmm, yes; in this scenario actually we'll autolaunch a bus first.  I guess checking X11 first is OK, though we could also simply skip looking at DBus if DBUS_SESSION_BUS_ADDRESS isn't set (a common way to avoid autolaunching).
Comment 13 Colin Walters 2011-03-18 14:46:08 UTC
Created attachment 183724 [details] [review]
bus: Rewrite a11y bus management, don't fall back to session bus

* Strip trailing newline
* Look up in X11 first for address
* Create libregistry-internals.la library for de-duplicating lookup
  logic (this would be much saner with non-recursive Make, but not
  going to do that now).
Comment 14 Mike Gorse 2011-03-18 15:58:28 UTC
Comment on attachment 183724 [details] [review]
bus: Rewrite a11y bus management, don't fall back to session bus

Do you see any reason not to rename/export the get_accessibility_bus function in atspi-misc.c and have at-spi2-registryd and libatk-bridge depend on libatspi?  At-spi2-atk currently has a duplicate function as well.  I'm not sure whether that would constitute an ABI break for instance.

Anyway, feel free to either commit what you have or make that change and commit--I'll probably be off-line until at least 5:00 PDT and don't want to hold things up...


>From 102a88778a2f05b3c651f5eda5f5f44fcf650c4b Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>Date: Tue, 15 Mar 2011 17:00:35 -0400
>Subject: [PATCH] bus: Rewrite a11y bus management, don't fall back to session bus
>
>First of all, there should *always* be an a11y bus; if I enable
>toolkit-accessibility, and apps pick that up but then libat-spi falls
>back to the session bus, that's broken.
>
>Fix this by rewriting the a11y bus launcher in C, making it a persistent
>session service, and giving it a DBus API (bus name: org.a11y.Bus).  It
>will start the bus process as a child dynamically; however if
>toolkit-accessibility is enabled, we start it on startup.  For more
>information, see the new file bus/README.
>
>Details:
>
>* Create a .service file so the session bus autostarts us if
>  necessary
>* The .desktop file is changed to use --start-immediately
>* Remove the kill_accessibility_bus() hack in registryd; instead
>  we chain at-spi-bus-launcher to the session bus lifecycle.
>* Change at-spi bus lookup to try both the new session bus API
>  and the X11 root window property.
>* Create libregistry-internals.la which encapsulates a11y bus
>  lookup logic, de-duplicating from atspi/ and registryd/.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=644851
>---
> atspi/Makefile.am                 |    2 +
> atspi/atspi-misc.c                |   70 +-------
> bus/Makefile.am                   |   30 ++-
> bus/README                        |   10 +
> bus/accessibility.conf            |    2 -
> bus/at-spi-bus-launcher.c         |  394 +++++++++++++++++++++++++++++++++++++
> bus/at-spi-dbus-bus.desktop.in    |    2 +-
> bus/at-spi-dbus-bus.in            |   13 --
> bus/org.a11y.Bus.service.in       |    3 +
> configure.ac                      |    5 +-
> registryd/Makefile.am             |   10 +
> registryd/libregistry-internals.c |  176 +++++++++++++++++
> registryd/libregistry-internals.h |   35 ++++
> registryd/registry-main.c         |  132 +------------
> 14 files changed, 658 insertions(+), 226 deletions(-)
> create mode 100644 bus/README
> create mode 100644 bus/at-spi-bus-launcher.c
> delete mode 100644 bus/at-spi-dbus-bus.in
> create mode 100644 bus/org.a11y.Bus.service.in
> create mode 100644 registryd/libregistry-internals.c
> create mode 100644 registryd/libregistry-internals.h
>
>diff --git a/atspi/Makefile.am b/atspi/Makefile.am
>index 569c6ff..1a50538 100644
>--- a/atspi/Makefile.am
>+++ b/atspi/Makefile.am
>@@ -4,10 +4,12 @@ libatspi_la_LDFLAGS = @LDFLAGS@ @LT_VERSION_INFO@ @LIBTOOL_EXPORT_OPTIONS@ -no-u
> 
> libatspi_la_CFLAGS = $(DBUS_GLIB_CFLAGS) \
> 		    $(DBIND_CFLAGS)     \
>+		    -I$(top_srcdir)/registryd \
>                     -I$(top_srcdir)
> 
> libatspi_la_LIBADD = $(DBUS_GLIB_LIBS) \
> 	$(X_LIBS) \
>+	$(top_builddir)/registryd/libregistry-internals.la \
> 	$(top_builddir)/dbind/libdbind.la
> 
> libatspiincludedir = $(includedir)/at-spi-2.0/atspi
>diff --git a/atspi/atspi-misc.c b/atspi/atspi-misc.c
>index 2adea05..584684d 100644
>--- a/atspi/atspi-misc.c
>+++ b/atspi/atspi-misc.c
>@@ -805,71 +805,6 @@ spi_display_name (void)
>   return canonical_display_name;
> }
> 
>-/* TODO: Avoid having duplicate functions for this here and in at-spi2-atk */
>-static DBusConnection *
>-get_accessibility_bus ()
>-{
>-  Atom AT_SPI_BUS;
>-  Atom actual_type;
>-  Display *bridge_display;
>-  int actual_format;
>-  unsigned char *data = NULL;
>-  unsigned long nitems;
>-  unsigned long leftover;
>-
>-  DBusConnection *bus = NULL;
>-  DBusError error;
>-
>-  bridge_display = XOpenDisplay (spi_display_name ());
>-  if (!bridge_display)
>-    {
>-      g_warning (_("AT-SPI: Could not get the display\n"));
>-      return NULL;
>-    }
>-
>-  AT_SPI_BUS = XInternAtom (bridge_display, "AT_SPI_BUS", False);
>-  XGetWindowProperty (bridge_display,
>-                      XDefaultRootWindow (bridge_display),
>-                      AT_SPI_BUS, 0L,
>-                      (long) BUFSIZ, False,
>-                      (Atom) 31, &actual_type, &actual_format,
>-                      &nitems, &leftover, &data);
>-  XCloseDisplay (bridge_display);
>-
>-  dbus_error_init (&error);
>-
>-  if (data == NULL)
>-    {
>-      g_warning
>-        (_("AT-SPI: Accessibility bus not found - Using session bus.\n"));
>-      bus = dbus_bus_get (DBUS_BUS_SESSION, &error);
>-      if (!bus)
>-        {
>-          g_warning (_("AT-SPI: Couldn't connect to bus: %s\n"), error.message);
>-          return NULL;
>-        }
>-    }
>-  else
>-    {
>-      bus = dbus_connection_open (data, &error);
>-      if (!bus)
>-        {
>-          g_warning (_("AT-SPI: Couldn't connect to bus: %s\n"), error.message);
>-          return NULL;
>-        }
>-      else
>-        {
>-          if (!dbus_bus_register (bus, &error))
>-            {
>-              g_warning (_("AT-SPI: Couldn't register with bus: %s\n"), error.message);
>-              return NULL;
>-            }
>-        }
>-    }
>-
>-  return bus;
>-}
>-
> /**
>  * atspi_init:
>  *
>@@ -895,12 +830,9 @@ atspi_init (void)
>   get_live_refs();
> 
>   dbus_error_init (&error);
>-  bus = get_accessibility_bus ();
>+  bus = _libregistry_get_a11y_bus ();
>   if (!bus)
>-  {
>-    g_warning ("Couldn't get session bus");
>     return 2;
>-  }
>   dbus_bus_register (bus, &error);
>   dbus_connection_setup_with_g_main(bus, g_main_context_default());
>   dbus_connection_add_filter (bus, atspi_dbus_filter, NULL, NULL);
>diff --git a/bus/Makefile.am b/bus/Makefile.am
>index 1f5a7d1..009d15a 100644
>--- a/bus/Makefile.am
>+++ b/bus/Makefile.am
>@@ -1,20 +1,30 @@
>+EXTRA_DIST=accessibility.conf
>+CLEANFILES=
>+
> busconfigdir = $(sysconfdir)/at-spi2
> busconfig_DATA = accessibility.conf
> 
>-atspidbusdir = $(libexecdir)
>-atspidbus_SCRIPTS = at-spi-dbus-bus
>+libexec_PROGRAMS = at-spi-bus-launcher
>+at_spi_bus_launcher_SOURCES = at-spi-bus-launcher.c
>+at_spi_bus_launcher_CPPFLAGS = -DSYSCONFDIR=\"$(sysconfdir)\" \
>+                               -DDBUS_DAEMON=\"$(DBUS_DAEMON)\"
>+at_spi_bus_launcher_CFLAGS = $(GIO_CFLAGS)
>+at_spi_bus_launcher_LDADD = $(GIO_LIBS) $(X_LIBS)
> 
> default_sessiondir = $(sysconfdir)/xdg/autostart
> default_session_DATA = at-spi-dbus-bus.desktop
> 
>+substitutions = "s,@libexecdir[@],$(libexecdir),"
> at-spi-dbus-bus.desktop: at-spi-dbus-bus.desktop.in
>-	sed -e "s,@libexecdir[@],$(libexecdir)," $< > $@.tmp && mv $@.tmp $@
>+	sed -e$ $(substitutions)  $< > $@.tmp && mv $@.tmp $@
>+EXTRA_DIST += at-spi-dbus-bus.desktop.in
>+CLEANFILES += at-spi-dbus-bus.desktop
>+
>+dbusservicedir=$(datadir)/dbus-1/services
>+dbusservice_DATA = org.a11y.Bus.service
> 
>-EXTRA_DIST= \
>-	accessibility.conf \
>-	at-spi-dbus-bus.in \
>-	at-spi-dbus-bus.desktop.in
>+org.a11y.Bus.service: org.a11y.Bus.service.in
>+	sed -e $(substitutions) $< > $@.tmp && mv $@.tmp $@
>+EXTRA_DIST += org.a11y.Bus.service.in
>+CLEANFILES += org.a11y.Bus.service
> 
>-CLEANFILES= \
>-	at-spi-dbus-bus.desktop \
>-	at-spi-dbus-bus
>diff --git a/bus/README b/bus/README
>new file mode 100644
>index 0000000..40b9ad6
>--- /dev/null
>+++ b/bus/README
>@@ -0,0 +1,10 @@
>+The a11y bus is accessed via two mechanisms:
>+
>+1) The DBus session bus, service "org.a11y.Bus", method "GetAddress")
>+2) The X11 root window property AT_SPI_BUS
>+
>+If the "toolkit-accessibility" variable is set, the bus is launched
>+immediately (and will be accessible immediately via the X11 property).
>+Otherwise, it will be spawned dynamically.
>+
>+
>diff --git a/bus/accessibility.conf b/bus/accessibility.conf
>index d9703e0..b9367d0 100644
>--- a/bus/accessibility.conf
>+++ b/bus/accessibility.conf
>@@ -3,8 +3,6 @@
> 
>   <type>accessibility</type>
> 
>-  <fork/>
>-
>   <standard_session_servicedirs/>
> 
>   <auth>EXTERNAL</auth>
>diff --git a/bus/at-spi-bus-launcher.c b/bus/at-spi-bus-launcher.c
>new file mode 100644
>index 0000000..b30f3c2
>--- /dev/null
>+++ b/bus/at-spi-bus-launcher.c
>@@ -0,0 +1,394 @@
>+/* -*- mode: c; c-basic-offset: 2; indent-tabs-mode: nil; -*-
>+ * 
>+ * at-spi-bus-launcher: Manage the a11y bus as a child process 
>+ *
>+ * Copyright 2011 Red Hat, Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Library General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Library General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Library General Public
>+ * License along with this library; if not, write to the
>+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>+ * Boston, MA 02111-1307, USA.
>+ */
>+
>+#include "config.h"
>+
>+#include <unistd.h>
>+#include <string.h>
>+#include <signal.h>
>+#include <sys/wait.h>
>+#include <errno.h>
>+
>+#include <gio/gio.h>
>+#include <X11/Xlib.h>
>+#include <X11/Xatom.h>
>+
>+typedef enum {
>+  A11Y_BUS_STATE_IDLE = 0,
>+  A11Y_BUS_STATE_READING_ADDRESS,
>+  A11Y_BUS_STATE_RUNNING,
>+  A11Y_BUS_STATE_ERROR
>+} A11yBusState;
>+
>+typedef struct {
>+  GMainLoop *loop;
>+  gboolean launch_immediately;
>+  GDBusConnection *session_bus;
>+
>+  A11yBusState state;
>+  /* -1 == error, 0 == pending, > 0 == running */
>+  int a11y_bus_pid;
>+  char *a11y_bus_address;
>+  int pipefd[2];
>+  char *a11y_launch_error_message;
>+} A11yBusLauncher;
>+
>+static A11yBusLauncher *_global_app = NULL;
>+
>+static const gchar introspection_xml[] =
>+  "<node>"
>+  "  <interface name='org.a11y.Bus'>"
>+  "    <method name='GetAddress'>"
>+  "      <arg type='s' name='address' direction='out'/>"
>+  "    </method>"
>+  "  </interface>"
>+  "</node>";
>+static GDBusNodeInfo *introspection_data = NULL;
>+
>+static void
>+setup_bus_child (gpointer data)
>+{
>+  A11yBusLauncher *app = data;
>+  (void) app;
>+
>+  close (app->pipefd[0]);
>+  dup2 (app->pipefd[1], 3);
>+  close (app->pipefd[1]);
>+
>+  /* On Linux, tell the bus process to exit if this process goes away */
>+#ifdef __linux
>+#include <sys/prctl.h>
>+  prctl (PR_SET_PDEATHSIG, 15);
>+#endif  
>+}
>+
>+/**
>+ * unix_read_all_fd_to_string:
>+ *
>+ * Read all data from a file descriptor to a C string buffer.
>+ */
>+static gboolean
>+unix_read_all_fd_to_string (int      fd,
>+                            char    *buf,
>+                            ssize_t  max_bytes)
>+{
>+  ssize_t bytes_read;
>+
>+  while (max_bytes > 1 && (bytes_read = read (fd, buf, MAX (4096, max_bytes - 1))))
>+    {
>+      if (bytes_read < 0)
>+        return FALSE;
>+      buf += bytes_read;
>+      max_bytes -= bytes_read;
>+    }
>+  *buf = '\0';
>+  return TRUE;
>+}
>+
>+static void
>+on_bus_exited (GPid     pid,
>+               gint     status,
>+               gpointer data)
>+{
>+  A11yBusLauncher *app = data;
>+  
>+  app->a11y_bus_pid = -1;
>+  app->state = A11Y_BUS_STATE_ERROR;
>+  if (app->a11y_launch_error_message == NULL)
>+    {
>+      if (WIFEXITED (status))
>+        app->a11y_launch_error_message = g_strdup_printf ("Bus exited with code %d", WEXITSTATUS (status));
>+      else if (WIFSIGNALED (status))
>+        app->a11y_launch_error_message = g_strdup_printf ("Bus killed by signal %d", WTERMSIG (status));
>+      else if (WIFSTOPPED (status))
>+        app->a11y_launch_error_message = g_strdup_printf ("Bus stopped by signal %d", WSTOPSIG (status));
>+    }
>+  g_main_loop_quit (app->loop);
>+} 
>+
>+static void
>+ensure_a11y_bus (A11yBusLauncher *app)
>+{
>+  GPid pid;
>+  char *argv[] = { DBUS_DAEMON, NULL, "--nofork", "--print-address", "3", NULL };
>+  char addr_buf[2048];
>+  GError *error = NULL;
>+
>+  if (app->a11y_bus_pid != 0)
>+    return;
>+  
>+  argv[1] = g_strdup_printf ("--config-file=%s/at-spi2/accessibility.conf", SYSCONFDIR);
>+
>+  if (pipe (app->pipefd) < 0)
>+    g_error ("Failed to create pipe: %s", strerror (errno));
>+  
>+  if (!g_spawn_async (NULL,
>+                      argv,
>+                      NULL,
>+                      G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD,
>+                      setup_bus_child,
>+                      app,
>+                      &pid,
>+                      &error))
>+    {
>+      app->a11y_bus_pid = -1;
>+      app->a11y_launch_error_message = g_strdup (error->message);
>+      g_clear_error (&error);
>+      goto error;
>+    }
>+
>+  close (app->pipefd[1]);
>+  app->pipefd[1] = -1;
>+
>+  g_child_watch_add (pid, on_bus_exited, app);
>+
>+  app->state = A11Y_BUS_STATE_READING_ADDRESS;
>+  app->a11y_bus_pid = pid;
>+  g_debug ("Launched a11y bus, child is %ld", (long) pid);
>+  if (!unix_read_all_fd_to_string (app->pipefd[0], addr_buf, sizeof (addr_buf)))
>+    {
>+      app->a11y_launch_error_message = g_strdup_printf ("Failed to read address: %s", strerror (errno));
>+      kill (app->a11y_bus_pid, SIGTERM);
>+      goto error;
>+    }
>+  close (app->pipefd[0]);
>+  app->pipefd[0] = -1;
>+  app->state = A11Y_BUS_STATE_RUNNING;
>+
>+  /* Trim the trailing newline */
>+  app->a11y_bus_address = g_strchomp (g_strdup (addr_buf));
>+
>+  {
>+    Display *display = XOpenDisplay (NULL);
>+    if (display)
>+      {
>+        Atom bus_address_atom = XInternAtom (display, "AT_SPI_BUS", False);
>+        XChangeProperty (display,
>+                         XDefaultRootWindow (display),
>+                         bus_address_atom,
>+                         XA_STRING, 8, PropModeReplace,
>+                         (guchar *) app->a11y_bus_address, strlen (app->a11y_bus_address));
>+      }
>+    XFlush (display);
>+    XCloseDisplay (display);
>+  }
>+
>+  return;
>+  
>+ error:
>+  close (app->pipefd[0]);
>+  close (app->pipefd[1]);
>+  app->state = A11Y_BUS_STATE_ERROR;
>+}
>+
>+static void
>+handle_method_call (GDBusConnection       *connection,
>+                    const gchar           *sender,
>+                    const gchar           *object_path,
>+                    const gchar           *interface_name,
>+                    const gchar           *method_name,
>+                    GVariant              *parameters,
>+                    GDBusMethodInvocation *invocation,
>+                    gpointer               user_data)
>+{
>+  A11yBusLauncher *app = user_data;
>+
>+  if (g_strcmp0 (method_name, "GetAddress") == 0)
>+    {
>+      ensure_a11y_bus (app);
>+      if (app->a11y_bus_pid > 0)
>+        g_dbus_method_invocation_return_value (invocation,
>+                                               g_variant_new ("(s)", app->a11y_bus_address));
>+      else
>+        g_dbus_method_invocation_return_dbus_error (invocation,
>+                                                    "org.a11y.Bus.Error",
>+                                                    app->a11y_launch_error_message);
>+    }
>+}
>+
>+static const GDBusInterfaceVTable interface_vtable =
>+{
>+  handle_method_call,
>+  NULL,
>+  NULL  /* handle_set_property */
>+};
>+
>+static void
>+on_bus_acquired (GDBusConnection *connection,
>+                 const gchar     *name,
>+                 gpointer         user_data)
>+{
>+  A11yBusLauncher *app = user_data;
>+  GError *error;
>+  guint registration_id;
>+  
>+  if (connection == NULL)
>+    {
>+      g_main_loop_quit (app->loop);
>+      return;
>+    }
>+  app->session_bus = connection;
>+
>+  if (app->launch_immediately)
>+    {
>+      ensure_a11y_bus (app);
>+      if (app->state == A11Y_BUS_STATE_ERROR)
>+        {
>+          g_main_loop_quit (app->loop);
>+          return;
>+        }
>+    }
>+
>+  error = NULL;
>+  registration_id = g_dbus_connection_register_object (connection,
>+                                                       "/org/a11y/bus",
>+                                                       introspection_data->interfaces[0],
>+                                                       &interface_vtable,
>+                                                       _global_app,
>+                                                       NULL,
>+                                                       &error);
>+  if (registration_id == 0)
>+    g_error ("%s", error->message);
>+}
>+
>+static void
>+on_name_lost (GDBusConnection *connection,
>+              const gchar     *name,
>+              gpointer         user_data)
>+{
>+  A11yBusLauncher *app = user_data;
>+  if (app->session_bus == NULL
>+      && connection == NULL
>+      && app->a11y_launch_error_message == NULL)
>+    app->a11y_launch_error_message = g_strdup ("Failed to connect to session bus");
>+  g_main_loop_quit (app->loop);
>+}
>+
>+static void
>+on_name_acquired (GDBusConnection *connection,
>+                  const gchar     *name,
>+                  gpointer         user_data)
>+{
>+  A11yBusLauncher *app = user_data;
>+  (void) app;
>+}
>+
>+static int sigterm_pipefd[2];
>+
>+static void
>+sigterm_handler (int signum)
>+{
>+  write (sigterm_pipefd[1], "X", 1);
>+}
>+
>+static gboolean
>+on_sigterm_pipe (GIOChannel  *channel,
>+                 GIOCondition condition,
>+                 gpointer     data)
>+{
>+  A11yBusLauncher *app = data;
>+  
>+  g_main_loop_quit (app->loop);
>+
>+  return FALSE;
>+}
>+
>+static void
>+init_sigterm_handling (A11yBusLauncher *app)
>+{
>+  GIOChannel *sigterm_channel;
>+
>+  if (pipe (sigterm_pipefd) < 0)
>+    g_error ("Failed to create pipe: %s", strerror (errno));
>+  signal (SIGTERM, sigterm_handler);
>+
>+  sigterm_channel = g_io_channel_unix_new (sigterm_pipefd[0]);
>+  g_io_add_watch (sigterm_channel,
>+                  G_IO_IN | G_IO_ERR | G_IO_HUP,
>+                  on_sigterm_pipe,
>+                  app);
>+}
>+
>+static gboolean
>+is_a11y_using_corba (void)
>+{
>+  char *gconf_argv[] = { "gconftool-2", "--get", "/desktop/gnome/interface/at-spi-corba", NULL };
>+  char *stdout = NULL;
>+  int estatus;
>+  gboolean result = FALSE;
>+
>+  if (!g_spawn_sync (NULL, gconf_argv, NULL,
>+                     G_SPAWN_SEARCH_PATH, NULL, NULL, &stdout, NULL, &estatus, NULL))
>+    goto out;
>+  if (estatus != 0)
>+    goto out;
>+  if (g_str_has_prefix (stdout, "true"))
>+    result = TRUE;
>+ out:
>+  g_free (stdout);
>+  return result;
>+}
>+
>+int
>+main (int    argc,
>+      char **argv)
>+{
>+  GError *error = NULL;
>+  GMainLoop *loop;
>+  GDBusConnection *session_bus;
>+  int name_owner_id;
>+
>+  g_type_init ();
>+
>+  if (is_a11y_using_corba ())
>+    return 0;
>+
>+  _global_app = g_slice_new0 (A11yBusLauncher);
>+  _global_app->loop = g_main_loop_new (NULL, FALSE);
>+  _global_app->launch_immediately = (argc == 2 && strcmp (argv[1], "--launch-immediately") == 0);
>+
>+  init_sigterm_handling (_global_app);
>+
>+  introspection_data = g_dbus_node_info_new_for_xml (introspection_xml, NULL);
>+  g_assert (introspection_data != NULL);
>+
>+  name_owner_id = g_bus_own_name (G_BUS_TYPE_SESSION,
>+                                  "org.a11y.Bus",
>+                                  G_BUS_NAME_OWNER_FLAGS_ALLOW_REPLACEMENT,
>+                                  on_bus_acquired,
>+                                  on_name_acquired,
>+                                  on_name_lost,
>+                                  _global_app,
>+                                  NULL);
>+
>+  g_main_loop_run (_global_app->loop);
>+
>+  if (_global_app->a11y_bus_pid > 0)
>+    kill (_global_app->a11y_bus_pid, SIGTERM);
>+    
>+  if (_global_app->a11y_launch_error_message)
>+    {
>+      g_printerr ("Failed to launch bus: %s", _global_app->a11y_launch_error_message);
>+      return 1;
>+    }
>+  return 0;
>+}
>diff --git a/bus/at-spi-dbus-bus.desktop.in b/bus/at-spi-dbus-bus.desktop.in
>index 8290478..0d25fda 100644
>--- a/bus/at-spi-dbus-bus.desktop.in
>+++ b/bus/at-spi-dbus-bus.desktop.in
>@@ -1,7 +1,7 @@
> [Desktop Entry]
> Type=Application
> Name=AT SPI D-Bus Bus
>-Exec=@libexecdir@/at-spi-dbus-bus
>+Exec=@libexecdir@/at-spi-bus-launcher --launch-immediately
> OnlyShowIn=GNOME;
> NoDisplay=true
> AutostartCondition=GSETTINGS org.gnome.desktop.interface toolkit-accessibility
>diff --git a/bus/at-spi-dbus-bus.in b/bus/at-spi-dbus-bus.in
>deleted file mode 100644
>index af92d25..0000000
>--- a/bus/at-spi-dbus-bus.in
>+++ /dev/null
>@@ -1,13 +0,0 @@
>-#!/bin/sh
>-
>-prefix="@prefix@"
>-sysconfdir="@sysconfdir@"
>-dbusdaemon="@DBUS_DAEMON@"
>-
>-var=`gconftool-2 --get /desktop/gnome/interface/at-spi-corba`
>-if [ "$var" == "true" ]; then
>-  exit 0
>-fi
>-
>-address=`${dbusdaemon} --config-file=${sysconfdir}/at-spi2/accessibility.conf --print-address 1 --print-pid 2 2>$HOME/.at-spi-dbus-bus.pid`
>-xprop -root -f AT_SPI_BUS 8s -set AT_SPI_BUS ${address}
>diff --git a/bus/org.a11y.Bus.service.in b/bus/org.a11y.Bus.service.in
>new file mode 100644
>index 0000000..60edc28
>--- /dev/null
>+++ b/bus/org.a11y.Bus.service.in
>@@ -0,0 +1,3 @@
>+[D-BUS Service]
>+Name=org.a11y.Bus
>+Exec=@libexecdir@/at-spi-bus-launcher
>diff --git a/configure.ac b/configure.ac
>index f5a581e..aee668c 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -55,6 +55,10 @@ PKG_CHECK_MODULES(GOBJ, [gobject-2.0 >= 2.0.0])
> AC_SUBST(GOBJ_LIBS)
> AC_SUBST(GOBJ_CFLAGS)
> 
>+PKG_CHECK_MODULES(GIO, [gio-2.0 >= 2.28])
>+AC_SUBST(GIO_LIBS)
>+AC_SUBST(GIO_CFLAGS)
>+
> # --------------------------------------------------------------------
> # Find DL functionality
> 
>@@ -192,7 +196,6 @@ AC_CONFIG_FILES([Makefile po/Makefile.in
> dbind/dbind-config.h
> 	atspi/Makefile
> 	registryd/Makefile
>-	bus/at-spi-dbus-bus
> 	bus/Makefile
> doc/Makefile
> doc/libatspi/Makefile
>diff --git a/registryd/Makefile.am b/registryd/Makefile.am
>index b4f929f..0d6a651 100644
>--- a/registryd/Makefile.am
>+++ b/registryd/Makefile.am
>@@ -1,4 +1,5 @@
> libexec_PROGRAMS = at-spi2-registryd
>+noinst_LTLIBRARIES = libregistry-internals.la
> 
> at_spi2_registryd_CFLAGS =	\
> 	$(GLIB_CFLAGS)		\
>@@ -8,7 +9,16 @@ at_spi2_registryd_CFLAGS =	\
> 	-I$(top_srcdir)		\
> 	-DATSPI_INTROSPECTION_PATH=\"$(pkgdatadir)/$(DEFAULT_ATSPI_INTROSPECTION_PATH)\"
> 
>+libregistry_internals_la_SOURCES = \
>+	libregistry-internals.h \
>+	libregistry-internals.c
>+
>+libregistry_internals_la_CFLAGS = $(DBUS_GLIB_CFLAGS)
>+libregistry_internals_la_LIBADD = $(DBUS_GLIB_LIBS) $(X_LIBS)
>+
>+
> at_spi2_registryd_LDADD =	\
>+	libregistry-internals.la \
> 	$(GLIB_LIBS)		\
> 	$(DBUS_GLIB_LIBS)	\
> 	$(GOBJ_CFLAGS)		\
>diff --git a/registryd/libregistry-internals.c b/registryd/libregistry-internals.c
>new file mode 100644
>index 0000000..722766c
>--- /dev/null
>+++ b/registryd/libregistry-internals.c
>@@ -0,0 +1,176 @@
>+/*
>+ * AT-SPI - Assistive Technology Service Provider Interface
>+ * (Gnome Accessibility Project; http://developer.gnome.org/projects/gap)
>+ *
>+ * Copyright 2001, 2002 Sun Microsystems Inc.,
>+ * Copyright 2001, 2002 Ximian, Inc.
>+ * Copyright 2011 Red Hat, Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Library General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Library General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Library General Public
>+ * License along with this library; if not, write to the
>+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>+ * Boston, MA 02111-1307, USA.
>+ */
>+
>+#include <config.h>
>+
>+#include "libregistry-internals.h"
>+#include <X11/Xlib.h>
>+#include <string.h>
>+#include <stdio.h>
>+
>+/*
>+ * Returns a 'canonicalized' value for DISPLAY,
>+ * with the screen number stripped off if present.
>+ *
>+ */
>+static const gchar*
>+spi_display_name (void)
>+{
>+    static const char *canonical_display_name = NULL;
>+    if (!canonical_display_name)
>+      {
>+        const gchar *display_env = g_getenv ("AT_SPI_DISPLAY");
>+        if (!display_env)
>+          {
>+            display_env = g_getenv ("DISPLAY");
>+            if (!display_env || !display_env[0]) 
>+                canonical_display_name = ":0";
>+            else
>+              {
>+                gchar *display_p, *screen_p;
>+                canonical_display_name = g_strdup (display_env);
>+                display_p = strrchr (canonical_display_name, ':');
>+                screen_p = strrchr (canonical_display_name, '.');
>+                if (screen_p && display_p && (screen_p > display_p))
>+                  {
>+                    *screen_p = '\0';
>+                  }
>+              }
>+          }
>+        else
>+          {
>+            canonical_display_name = display_env;
>+          }
>+      }
>+    return canonical_display_name;
>+}
>+
>+/*
>+ * Gets the IOR from the XDisplay.
>+ */
>+static char *
>+get_accessibility_bus_address_x11 (void)
>+{
>+  Atom AT_SPI_BUS;
>+  Atom actual_type;
>+  Display *bridge_display;
>+  int actual_format;
>+  unsigned char *data = NULL;
>+  unsigned long nitems;
>+  unsigned long leftover;
>+
>+  bridge_display = XOpenDisplay (spi_display_name ());
>+  if (!bridge_display)
>+    {
>+      g_warning ("Could not open X display");
>+      return NULL;
>+    }
>+      
>+  AT_SPI_BUS = XInternAtom (bridge_display, "AT_SPI_BUS", False);
>+  XGetWindowProperty (bridge_display,
>+		      XDefaultRootWindow (bridge_display),
>+		      AT_SPI_BUS, 0L,
>+		      (long) BUFSIZ, False,
>+		      (Atom) 31, &actual_type, &actual_format,
>+		      &nitems, &leftover, &data);
>+  XCloseDisplay (bridge_display);
>+
>+  return g_strdup (data);
>+}
>+
>+static char *
>+get_accessibility_bus_address_dbus (void)
>+{
>+  DBusConnection *session_bus = NULL;
>+  DBusMessage *message;
>+  DBusMessage *reply;
>+  char *address = NULL;
>+
>+  session_bus = dbus_bus_get (DBUS_BUS_SESSION, NULL);
>+  if (!session_bus)
>+    return NULL;
>+
>+  message = dbus_message_new_method_call ("org.a11y.Bus",
>+					  "/org/a11y/bus",
>+					  "org.a11y.Bus",
>+					  "GetAddress");
>+
>+  reply = dbus_connection_send_with_reply_and_block (session_bus,
>+						     message,
>+						     -1,
>+						     NULL);
>+  dbus_message_unref (message);
>+
>+  if (!reply)
>+    return NULL;
>+  
>+  {
>+    const char *tmp_address;
>+    if (!dbus_message_get_args (reply,
>+				NULL,
>+				DBUS_TYPE_STRING,
>+				&tmp_address,
>+				DBUS_TYPE_INVALID))
>+      {
>+	dbus_message_unref (reply);
>+	return NULL;
>+      }
>+    address = g_strdup (tmp_address);
>+    dbus_message_unref (reply);
>+  }
>+  
>+  return address;
>+}
>+
>+DBusConnection *
>+_libregistry_get_a11y_bus (void)
>+{
>+  DBusConnection *bus = NULL;
>+  DBusError error;
>+  char *address;
>+
>+  address = get_accessibility_bus_address_x11 ();
>+  if (!address)
>+    address = get_accessibility_bus_address_dbus ();
>+  if (!address)
>+    return NULL;
>+
>+  dbus_error_init (&error);
>+  bus = dbus_connection_open (address, &error);
>+  if (!bus)
>+    {
>+      g_warning ("Couldn't connect to accessibility bus: %s", error.message);
>+      return NULL;
>+    }
>+  else
>+    {
>+      if (!dbus_bus_register (bus, &error))
>+	{
>+	  g_warning ("Couldn't register with accessibility bus: %s", error.message);
>+	  return NULL;
>+	}
>+    }
>+  
>+  return bus;
>+}
>diff --git a/registryd/libregistry-internals.h b/registryd/libregistry-internals.h
>new file mode 100644
>index 0000000..66184f8
>--- /dev/null
>+++ b/registryd/libregistry-internals.h
>@@ -0,0 +1,35 @@
>+/*
>+ * AT-SPI - Assistive Technology Service Provider Interface
>+ * (Gnome Accessibility Project; http://developer.gnome.org/projects/gap)
>+ *
>+ * Copyright 2011 Red Hat, Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Library General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Library General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Library General Public
>+ * License along with this library; if not, write to the
>+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>+ * Boston, MA 02111-1307, USA.
>+ */
>+
>+#ifndef __LIBREGISTRY_INTERNALS_H__
>+#define __LIBREGISTRY_INTERNALS_H__ 
>+
>+#include <dbus/dbus-glib.h>
>+#include <dbus/dbus.h>
>+
>+G_BEGIN_DECLS
>+
>+DBusConnection *_libregistry_get_a11y_bus (void);
>+
>+G_END_DECLS
>+
>+#endif /* __LIBREGISTRY_INTERNALS_H__ */
>diff --git a/registryd/registry-main.c b/registryd/registry-main.c
>index 083de19..d35475b 100644
>--- a/registryd/registry-main.c
>+++ b/registryd/registry-main.c
>@@ -34,6 +34,7 @@
> #include "paths.h"
> #include "registry.h"
> #include "deviceeventcontroller.h"
>+#include "libregistry-internals.h"
> 
> #define CORBA_GCONF_KEY  "/desktop/gnome/interface/at-spi-corba"
> 
>@@ -88,36 +89,8 @@ session_manager_connect (void)
> }
> 
> static void
>-kill_accessibility_bus ()
>-{
>-  FILE *fp;
>-  const char *home;
>-  char *name;
>-  int pid;
>-
>-  home = getenv ("HOME");
>-  if (!home)
>-    return;
>-  name = g_strconcat (home, "/", ".atspi-dbus-bus.pid", NULL);
>-  if (!name)
>-    return;
>-
>-  fp = fopen (name, "r");
>-  if (fp)
>-  {
>-    if (fscanf (fp, "%d", &pid) == 1)
>-    {
>-      kill (&pid, SIGTERM);
>-    }
>-    fclose (fp);
>-  }
>-  g_free (name);
>-}
>-
>-static void
> stop_cb (gpointer data)
> {
>-        kill_accessibility_bus ();
>         g_main_loop_quit (mainloop);
> }
> 
>@@ -151,7 +124,6 @@ query_end_session_cb (guint flags, gpointer data)
> static void
> end_session_cb (guint flags, gpointer data)
> {
>-        kill_accessibility_bus ();
>         end_session_response (TRUE, NULL);
>         g_main_loop_quit (mainloop);
> }
>@@ -205,105 +177,6 @@ register_client (void)
> 
> /*---------------------------------------------------------------------------*/
> 
>-/*
>- * Returns a 'canonicalized' value for DISPLAY,
>- * with the screen number stripped off if present.
>- *
>- */
>-static const gchar*
>-spi_display_name (void)
>-{
>-    static const char *canonical_display_name = NULL;
>-    if (!canonical_display_name)
>-      {
>-        const gchar *display_env = g_getenv ("AT_SPI_DISPLAY");
>-        if (!display_env)
>-          {
>-            display_env = g_getenv ("DISPLAY");
>-            if (!display_env || !display_env[0]) 
>-                canonical_display_name = ":0";
>-            else
>-              {
>-                gchar *display_p, *screen_p;
>-                canonical_display_name = g_strdup (display_env);
>-                display_p = strrchr (canonical_display_name, ':');
>-                screen_p = strrchr (canonical_display_name, '.');
>-                if (screen_p && display_p && (screen_p > display_p))
>-                  {
>-                    *screen_p = '\0';
>-                  }
>-              }
>-          }
>-        else
>-          {
>-            canonical_display_name = display_env;
>-          }
>-      }
>-    return canonical_display_name;
>-}
>-
>-/*---------------------------------------------------------------------------*/
>-
>-/*
>- * Gets the IOR from the XDisplay.
>- * Not currently used in D-Bus version, but something similar
>- * may be employed in the future for accessing the registry daemon
>- * bus name.
>- */
>-
>-static DBusConnection *
>-spi_get_bus (void)
>-{
>-     Atom AT_SPI_BUS;
>-     Atom actual_type;
>-     Display *bridge_display;
>-     int actual_format;
>-     unsigned char *data = NULL;  
>-     unsigned long nitems;
>-     unsigned long leftover;
>-
>-     DBusConnection *bus = NULL;
>-     DBusError       error;
>-
>-     bridge_display = XOpenDisplay (spi_display_name ());
>-     if (!bridge_display)
>-	g_error ("AT_SPI: Could not get the display");
>-
>-     AT_SPI_BUS = XInternAtom (bridge_display, "AT_SPI_BUS", FALSE); 
>-     XGetWindowProperty(bridge_display, 
>-                        XDefaultRootWindow (bridge_display),
>-                        AT_SPI_BUS, 0L,
>-                        (long)BUFSIZ, False,
>-                        (Atom) 31, &actual_type, &actual_format,
>-                        &nitems, &leftover, &data);
>-
>-     dbus_error_init (&error);
>-
>-     if (data == NULL)
>-     {
>-         g_warning ("AT-SPI: Accessibility bus bus not found - Using session bus.\n");
>-         bus = dbus_bus_get (DBUS_BUS_SESSION, &error);
>-         if (!bus)
>-             g_error ("AT-SPI: Couldn't connect to bus: %s\n", error.message);
>-     }
>-     else
>-     {
>-	 bus = dbus_connection_open (data, &error);
>-	 XFree (data);
>-         if (!bus)
>-         {
>-             g_error ("AT-SPI: Couldn't connect to bus: %s\n", error.message);
>-         }
>-	 else
>-         {
>-	     if (!dbus_bus_register (bus, &error))
>-	         g_error ("AT-SPI: Couldn't register with bus: %s\n", error.message);
>-         } 
>-     }
>-
>-  XCloseDisplay (bridge_display);
>-     return bus;
>-}
> 
> /*---------------------------------------------------------------------------*/
> 
>@@ -340,8 +213,7 @@ main (int argc, char **argv)
>       dbus_name = SPI_DBUS_NAME_REGISTRY;
> 
>   dbus_error_init (&error);
>-  bus = dbus_bus_get(DBUS_BUS_SESSION, &error);
>-  bus = spi_get_bus ();
>+  bus = _libregistry_get_a11y_bus ();
>   if (!bus)
>   {
>     return 0;
>-- 
>1.7.4.1
Comment 15 Colin Walters 2011-03-18 16:02:14 UTC
(In reply to comment #14)
> (From update of attachment 183724 [details] [review])
> Do you see any reason not to rename/export the get_accessibility_bus function
> in atspi-misc.c and have at-spi2-registryd and libatk-bridge depend on
> libatspi?  At-spi2-atk currently has a duplicate function as well.  I'm not
> sure whether that would constitute an ABI break for instance.

Hmm; to be honest I am not totally sure I understand all the moving pieces here (I'm gaining some of that knowledge), but how e.g. at-spi2-atk fits in isn't one of them yet.

It feels like the bus launcher is duplicative with "registryd" in a lot of ways?

Ok, let's go ahead with this and get some testing; we can fix up at-spi2-atk later, it should still work fine just looking at X11.
 
> Anyway, feel free to either commit what you have or make that change and
> commit--I'll probably be off-line until at least 5:00 PDT and don't want to
> hold things up...
> 
> 
> >From 102a88778a2f05b3c651f5eda5f5f44fcf650c4b Mon Sep 17 00:00:00 2001

Btw can you avoid editing the whole patch as text for reply?  Quoting it in its entirety is making this bug very loooong =)  The best is to use the "Review" tool.
Comment 16 Colin Walters 2011-03-18 16:31:46 UTC
Attachment 183724 [details] pushed as ce599c1 - bus: Rewrite a11y bus management, don't fall back to session bus
Comment 17 Colin Walters 2011-03-18 16:33:25 UTC
Pushed to master; let's follow up with any additional fixes as separate bugs/commits.