2003-03-16 Havoc Pennington <hp@pobox.com>

	Oops - test code was only testing failure of around 30 of the
	mallocs in the test path, but it turns out there are 500+
	mallocs. I believe this was due to misguided linking setup such
	that there was one copy of dbus_malloc etc. in the daemon and one
	in the shared lib, and only daemon mallocs were tested. In any
	case, the test case now tests all 500+ mallocs, and doesn't pass
	yet, though there are lots of fixes in this patch.

	* dbus/dbus-connection.c (dbus_connection_dispatch_message): fix
	this so that it doesn't need to allocate memory, since it
	has no way of indicating failure due to OOM (and would be
	annoying if it did).

	* dbus/dbus-list.c (_dbus_list_pop_first_link): new function

	* bus/Makefile.am: rearrange to create two self-contained
	libraries, to avoid having libraries with overlapping symbols.
	that was resulting in weirdness, e.g. I'm pretty sure there
	were two copies of global static variables.

	* dbus/dbus-internals.c: move the malloc debug stuff to
	dbus-memory.c

	* dbus/dbus-list.c (free_link): free list mempool if it becomes
	empty.

	* dbus/dbus-memory.c (_dbus_disable_mem_pools): new function

	* dbus/dbus-address.c (dbus_parse_address): free list nodes
	on failure.

	* bus/dispatch.c (bus_dispatch_add_connection): free
	message_handler_slot when no longer using it, so
	memory leak checkers are happy for the test suite.

	* dbus/dbus-server-debug-pipe.c (debug_finalize): free server name

	* bus/bus.c (new_connection_callback): disconnect in here if
	bus_connections_setup_connection fails.

	* bus/connection.c (bus_connections_unref): fix to free the
	connections
	(bus_connections_setup_connection): if this fails, don't
	disconnect the connection, just be sure there are no side
	effects.

	* dbus/dbus-string.c (undo_alignment): unbreak this

	* dbus/dbus-auth.c (_dbus_auth_unref): free some stuff we were
	leaking
	(_dbus_auth_new): fix the order in which we free strings
	on OOM failure

	* bus/connection.c (bus_connection_disconnected): fix to
	not send ServiceDeleted multiple times in case of memory
	allocation failure

	* dbus/dbus-bus.c (dbus_bus_get_base_service): new function to
	get the base service name
	(dbus_bus_register_client): don't return base service name,
	instead store it on the DBusConnection and have an accessor
	function for it.
	(dbus_bus_register_client): rename dbus_bus_register()

	* bus/dispatch.c (check_hello_message): verify that other
	connections on the bus also got the correct results, not
	just the one sending hello

6 files changed, 586 insertions, 110 deletions

diff --git a/bus/Makefile.am b/bus/Makefile.am
index 9c2f08bd..aee6e37a 100644
--- a/bus/Makefile.am
+++ b/bus/Makefile.am
@@ -6,9 +6,7 @@ EFENCE=
 
 bin_PROGRAMS=dbus-daemon-1
 
-noinst_LTLIBRARIES=libdbus-daemon.la
-
-libdbus_daemon_la_SOURCES=			\
+BUS_SOURCES=					\
 	activation.c				\
 	activation.h				\
 	bus.c					\
@@ -30,18 +28,14 @@ libdbus_daemon_la_SOURCES=			\
 	utils.c					\
 	utils.h
 
-
-libdbus_daemon_la_LIBADD=				\
-	$(top_builddir)/dbus/libdbus-convenience.la
-
 dbus_daemon_1_SOURCES=				\
+	$(BUS_SOURCES)				\
 	main.c					
 
 dbus_daemon_1_LDADD=					\
 	$(EFENCE)					\
 	$(DBUS_BUS_LIBS)				\
-	$(top_builddir)/bus/libdbus-daemon.la		\
-	$(top_builddir)/dbus/libdbus-1.la
+	$(top_builddir)/dbus/libdbus-convenience.la
 
 ## note that TESTS has special meaning (stuff to use in make check)
 ## so if adding tests not to be run in make check, don't add them to 
@@ -58,9 +52,10 @@ endif
 noinst_PROGRAMS=$(TESTS)
 
 bus_test_SOURCES=				\
+	$(BUS_SOURCES)				\
 	test-main.c
 
-bus_test_LDADD= $(top_builddir)/dbus/libdbus-1.la libdbus-daemon.la
+bus_test_LDADD=$(top_builddir)/dbus/libdbus-convenience.la
 
 ## mop up the gcov files
 clean-local:
diff --git a/bus/bus.c b/bus/bus.c
index 2270f534..4319e5fe 100644
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -94,8 +94,17 @@ new_connection_callback (DBusServer     *server,
   BusContext *context = data;
   
   if (!bus_connections_setup_connection (context->connections, new_connection))
-    _dbus_verbose ("No memory to setup new connection\n");
+    {
+      _dbus_verbose ("No memory to setup new connection\n");
 
+      /* if we don't do this, it will get unref'd without
+       * being disconnected... kind of strange really
+       * that we have to do this, people won't get it right
+       * in general.
+       */
+      dbus_connection_disconnect (new_connection);
+    }
+  
   /* on OOM, we won't have ref'd the connection so it will die. */
 }
 
@@ -223,16 +232,34 @@ bus_context_unref (BusContext *context)
 
   if (context->refcount == 0)
     {
+      _dbus_verbose ("Finalizing bus context %p\n", context);
+      
       bus_context_shutdown (context);
+
+      if (context->connections)
+        {
+          bus_connections_unref (context->connections);
+          context->connections = NULL;
+        }
       
       if (context->registry)
-        bus_registry_unref (context->registry);
-      if (context->connections)
-        bus_connections_unref (context->connections);
+        {
+          bus_registry_unref (context->registry);
+          context->registry = NULL;
+        }
+      
       if (context->activation)
-        bus_activation_unref (context->activation);
+        {
+          bus_activation_unref (context->activation);
+          context->activation = NULL;
+        }
+      
       if (context->server)
-        dbus_server_unref (context->server);
+        {
+          dbus_server_unref (context->server);
+          context->server = NULL;
+        }
+      
       dbus_free (context->address);
       dbus_free (context);
     }
diff --git a/bus/connection.c b/bus/connection.c
index ee3612ae..3308df0f 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -70,38 +70,38 @@ bus_connection_disconnected (DBusConnection *connection)
    * stuff, not just sending a message (so we can e.g. revert
    * removal of service owners).
    */
-  {
-    BusTransaction *transaction;
-    DBusError error;
-
-    dbus_error_init (&error);
-
-    transaction = NULL;
-    while (transaction == NULL)
-      {
-        transaction = bus_transaction_new (d->connections->context);
-        bus_wait_for_memory ();
-      }
-    
-    while ((service = _dbus_list_get_last (&d->services_owned)))
-      {
-      retry:
-        if (!bus_service_remove_owner (service, connection,
-                                       transaction, &error))
-          {
-            if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
-              {
-                dbus_error_free (&error);
-                bus_wait_for_memory ();
-                goto retry;
-              }
-            else
-              _dbus_assert_not_reached ("Removing service owner failed for non-memory-related reason");
-          }
-      }
-
-    bus_transaction_execute_and_free (transaction);
-  }
+  while ((service = _dbus_list_get_last (&d->services_owned)))
+    {
+      BusTransaction *transaction;
+      DBusError error;
+
+    retry:
+      
+      dbus_error_init (&error);
+        
+      transaction = NULL;
+      while (transaction == NULL)
+        {
+          transaction = bus_transaction_new (d->connections->context);
+          bus_wait_for_memory ();
+        }
+        
+      if (!bus_service_remove_owner (service, connection,
+                                     transaction, &error))
+        {
+          if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
+            {
+              dbus_error_free (&error);
+              bus_transaction_cancel_and_free (transaction);
+              bus_wait_for_memory ();
+              goto retry;
+            }
+          else
+            _dbus_assert_not_reached ("Removing service owner failed for non-memory-related reason");
+        }
+        
+      bus_transaction_execute_and_free (transaction);
+    }
 
   bus_dispatch_remove_connection (connection);
   
@@ -247,8 +247,17 @@ bus_connections_unref (BusConnections *connections)
   connections->refcount -= 1;
   if (connections->refcount == 0)
     {
-      /* FIXME free each connection... */
-      _dbus_assert_not_reached ("shutting down connections not implemented");
+      while (connections->list != NULL)
+        {
+          DBusConnection *connection;
+
+          connection = connections->list->data;
+
+          dbus_connection_ref (connection);
+          dbus_connection_disconnect (connection);
+          bus_connection_disconnected (connection);
+          dbus_connection_unref (connection);
+        }
       
       _dbus_list_clear (&connections->list);
       
@@ -261,7 +270,8 @@ bus_connections_setup_connection (BusConnections *connections,
                                   DBusConnection *connection)
 {
   BusConnectionData *d;
-
+  dbus_bool_t retval;
+  
   d = dbus_new0 (BusConnectionData, 1);
   
   if (d == NULL)
@@ -277,13 +287,8 @@ bus_connections_setup_connection (BusConnections *connections,
       dbus_free (d);
       return FALSE;
     }
-  
-  if (!_dbus_list_append (&connections->list, connection))
-    {
-      /* this will free our data when connection gets finalized */
-      dbus_connection_disconnect (connection);
-      return FALSE;
-    }
+
+  retval = FALSE;
   
   if (!dbus_connection_set_watch_functions (connection,
                                             (DBusAddWatchFunction) add_connection_watch,
@@ -291,32 +296,46 @@ bus_connections_setup_connection (BusConnections *connections,
                                             NULL,
                                             connection,
                                             NULL))
-    {
-      dbus_connection_disconnect (connection);
-      return FALSE;
-    }
+    goto out;
   
   if (!dbus_connection_set_timeout_functions (connection,
                                               (DBusAddTimeoutFunction) add_connection_timeout,
                                               (DBusRemoveTimeoutFunction) remove_connection_timeout,
                                               NULL,
                                               connection, NULL))
-    {
-      dbus_connection_disconnect (connection);
-      return FALSE;
-    }
+    goto out;
 
   
   /* Setup the connection with the dispatcher */
   if (!bus_dispatch_add_connection (connection))
+    goto out;
+  
+  if (!_dbus_list_append (&connections->list, connection))
     {
-      dbus_connection_disconnect (connection);
-      return FALSE;
+      bus_dispatch_remove_connection (connection);
+      goto out;
     }
-
+  
   dbus_connection_ref (connection);
+  retval = TRUE;
+
+ out:
+  if (!retval)
+    {
+      if (!dbus_connection_set_watch_functions (connection,
+                                                NULL, NULL, NULL,
+                                                connection,
+                                                NULL))
+        _dbus_assert_not_reached ("setting watch functions to NULL failed");
+      
+      if (!dbus_connection_set_timeout_functions (connection,
+                                                  NULL, NULL, NULL,
+                                                  connection,
+                                                  NULL))
+        _dbus_assert_not_reached ("setting timeout functions to NULL failed");
+    }
   
-  return TRUE;
+  return retval;
 }
 
 
@@ -607,8 +626,10 @@ bus_transaction_send_message (BusTransaction *transaction,
   BusConnectionData *d;
   DBusList *link;
 
-  _dbus_verbose ("  trying to add message %s to transaction\n",
-                 dbus_message_get_name (message));
+  _dbus_verbose ("  trying to add message %s to transaction%s\n",
+                 dbus_message_get_name (message),
+                 dbus_connection_get_is_connected (connection) ?
+                 "" : " (disconnected)");
   
   if (!dbus_connection_get_is_connected (connection))
     return TRUE; /* silently ignore disconnected connections */
@@ -702,6 +723,8 @@ void
 bus_transaction_cancel_and_free (BusTransaction *transaction)
 {
   DBusConnection *connection;
+
+  _dbus_verbose ("TRANSACTION: cancelled\n");
   
   while ((connection = _dbus_list_pop_first (&transaction->connections)))
     connection_cancel_transaction (connection, transaction);
@@ -754,6 +777,8 @@ bus_transaction_execute_and_free (BusTransaction *transaction)
    * send the messages
    */
   DBusConnection *connection;
+
+  _dbus_verbose ("TRANSACTION: executing\n");
   
   while ((connection = _dbus_list_pop_first (&transaction->connections)))
     connection_execute_transaction (connection, transaction);
@@ -796,9 +821,6 @@ bus_transaction_send_error_reply (BusTransaction  *transaction,
 
   _dbus_assert (error != NULL);
   _DBUS_ASSERT_ERROR_IS_SET (error);
-
-  _dbus_verbose ("  trying to add error %s to transaction\n",
-                 error->name);
   
   reply = dbus_message_new_error_reply (in_reply_to,
                                         error->name,
diff --git a/bus/dispatch.c b/bus/dispatch.c
index d0def78f..0fe4be19 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -2,6 +2,7 @@
 /* dispatch.c  Message dispatcher
  *
  * Copyright (C) 2003  CodeFactory AB
+ * Copyright (C) 2003  Red Hat, Inc.
  *
  * Licensed under the Academic Free License version 1.2
  * 
@@ -33,6 +34,7 @@
 #include <string.h>
 
 static int message_handler_slot;
+static int message_handler_slot_refcount;
 
 typedef struct
 {
@@ -329,22 +331,46 @@ bus_dispatch_message_handler (DBusMessageHandler *handler,
   return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS;
 }
 
-dbus_bool_t
-bus_dispatch_add_connection (DBusConnection *connection)
+static dbus_bool_t
+message_handler_slot_ref (void)
 {
-  DBusMessageHandler *handler;
-  
   message_handler_slot = dbus_connection_allocate_data_slot ();
 
   if (message_handler_slot < 0)
     return FALSE;
 
+  message_handler_slot_refcount += 1;
+
+  return TRUE;
+}
+
+static void
+message_handler_slot_unref (void)
+{
+  _dbus_assert (message_handler_slot_refcount > 0);
+  message_handler_slot_refcount -= 1;
+  if (message_handler_slot_refcount == 0)
+    {
+      dbus_connection_free_data_slot (message_handler_slot);
+      message_handler_slot = -1;
+    }
+}
+
+dbus_bool_t
+bus_dispatch_add_connection (DBusConnection *connection)
+{
+  DBusMessageHandler *handler;
+
+  if (!message_handler_slot_ref ())
+    return FALSE;
+  
   handler = dbus_message_handler_new (bus_dispatch_message_handler, NULL, NULL);  
 
   if (!dbus_connection_add_filter (connection, handler))
     {
       dbus_message_handler_unref (handler);
-
+      message_handler_slot_unref ();
+      
       return FALSE;
     }
 
@@ -355,6 +381,7 @@ bus_dispatch_add_connection (DBusConnection *connection)
     {
       dbus_connection_remove_filter (connection, handler);
       dbus_message_handler_unref (handler);
+      message_handler_slot_unref ();
 
       return FALSE;
     }
@@ -371,9 +398,9 @@ bus_dispatch_remove_connection (DBusConnection *connection)
   dbus_connection_set_data (connection,
 			    message_handler_slot,
 			    NULL, NULL);
-}
-
 
+  message_handler_slot_unref ();
+}
 
 #ifdef DBUS_BUILD_TESTS
 
@@ -381,6 +408,8 @@ typedef dbus_bool_t (* Check1Func) (BusContext     *context);
 typedef dbus_bool_t (* Check2Func) (BusContext     *context,
                                     DBusConnection *connection);
 
+static dbus_bool_t check_no_leftovers (BusContext *context);
+
 static void
 flush_bus (BusContext *context)
 {  
@@ -388,13 +417,239 @@ flush_bus (BusContext *context)
     ;
 }
 
+typedef struct
+{
+  const char *expected_service_name;
+  dbus_bool_t failed;
+} CheckServiceDeletedData;
+
+static dbus_bool_t
+check_service_deleted_foreach (DBusConnection *connection,
+                               void           *data)
+{
+  CheckServiceDeletedData *d = data;
+  DBusMessage *message;
+  DBusError error;
+  char *service_name;
+
+  dbus_error_init (&error);
+  d->failed = TRUE;
+  service_name = NULL;
+  
+  message = dbus_connection_pop_message (connection);
+  if (message == NULL)
+    {
+      _dbus_warn ("Did not receive a message on %p, expecting %s\n",
+                  connection, DBUS_MESSAGE_SERVICE_DELETED);
+      goto out;
+    }
+  else if (!dbus_message_name_is (message, DBUS_MESSAGE_SERVICE_DELETED))
+    {
+      _dbus_warn ("Received message %s on %p, expecting %s\n",
+                  dbus_message_get_name (message),
+                  connection, DBUS_MESSAGE_SERVICE_DELETED);
+      goto out;
+    }
+  else
+    {
+      if (!dbus_message_get_args (message, &error,
+                                  DBUS_TYPE_STRING, &service_name,
+                                  DBUS_TYPE_INVALID))
+        {
+          if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
+            {
+              _dbus_verbose ("no memory to get service name arg\n");
+            }
+          else
+            {
+              _dbus_assert (dbus_error_is_set (&error));
+              _dbus_warn ("Did not get the expected single string argument\n");
+              goto out;
+            }
+        }
+      else if (strcmp (service_name, d->expected_service_name) != 0)
+        {
+          _dbus_warn ("expected deletion of service %s, got deletion of %s\n",
+                      d->expected_service_name,
+                      service_name);
+          goto out;
+        }
+    }
+
+  d->failed = FALSE;
+  
+ out:
+  dbus_free (service_name);
+  dbus_error_free (&error);
+  
+  if (message)
+    dbus_message_unref (message);
+
+  return !d->failed;
+}
+
 static void
-kill_client_connection (DBusConnection *connection)
+kill_client_connection (BusContext     *context,
+                        DBusConnection *connection)
 {
+  char *base_service;
+  const char *s;
+  CheckServiceDeletedData csdd;
+
+  _dbus_verbose ("killing connection %p\n", connection);
+  
+  s = dbus_bus_get_base_service (connection);
+  _dbus_assert (s != NULL);
+
+  while ((base_service = _dbus_strdup (s)) == NULL)
+    bus_wait_for_memory ();
+
+  dbus_connection_ref (connection);
+  
   /* kick in the disconnect handler that unrefs the connection */
   dbus_connection_disconnect (connection);
-  while (dbus_connection_dispatch_message (connection))
-    ;
+
+  flush_bus (context);
+
+  _dbus_assert (bus_test_client_listed (connection));
+  
+  /* Run disconnect handler in test.c */
+  if (dbus_connection_dispatch_message (connection))
+    _dbus_assert_not_reached ("something received on connection being killed other than the disconnect");
+  
+  _dbus_assert (!dbus_connection_get_is_connected (connection));
+  dbus_connection_unref (connection);
+  connection = NULL;
+  _dbus_assert (!bus_test_client_listed (connection));
+  
+  csdd.expected_service_name = base_service;
+  csdd.failed = FALSE;
+
+  bus_test_clients_foreach (check_service_deleted_foreach,
+                            &csdd);
+
+  dbus_free (base_service);
+  
+  if (csdd.failed)
+    _dbus_assert_not_reached ("didn't get the expected ServiceDeleted messages");
+  
+  if (!check_no_leftovers (context))
+    _dbus_assert_not_reached ("stuff left in message queues after disconnecting a client");
+}
+
+typedef struct
+{
+  dbus_bool_t failed;
+} CheckNoMessagesData;
+
+static dbus_bool_t
+check_no_messages_foreach (DBusConnection *connection,
+                           void           *data)
+{
+  CheckNoMessagesData *d = data;
+  DBusMessage *message;
+
+  message = dbus_connection_pop_message (connection);
+  if (message != NULL)
+    {
+      _dbus_warn ("Received message %s on %p, expecting no messages\n",
+                  dbus_message_get_name (message), connection);
+      d->failed = TRUE;
+    }
+
+  if (message)
+    dbus_message_unref (message);
+  return !d->failed;
+}
+
+typedef struct
+{
+  DBusConnection *skip_connection;
+  const char *expected_service_name;
+  dbus_bool_t failed;
+} CheckServiceCreatedData;
+
+static dbus_bool_t
+check_service_created_foreach (DBusConnection *connection,
+                               void           *data)
+{
+  CheckServiceCreatedData *d = data;
+  DBusMessage *message;
+  DBusError error;
+  char *service_name;
+
+  if (connection == d->skip_connection)
+    return TRUE;
+
+  dbus_error_init (&error);
+  d->failed = TRUE;
+  service_name = NULL;
+  
+  message = dbus_connection_pop_message (connection);
+  if (message == NULL)
+    {
+      _dbus_warn ("Did not receive a message on %p, expecting %s\n",
+                  connection, DBUS_MESSAGE_SERVICE_CREATED);
+      goto out;
+    }
+  else if (!dbus_message_name_is (message, DBUS_MESSAGE_SERVICE_CREATED))
+    {
+      _dbus_warn ("Received message %s on %p, expecting %s\n",
+                  dbus_message_get_name (message),
+                  connection, DBUS_MESSAGE_SERVICE_CREATED);
+      goto out;
+    }
+  else
+    {
+      if (!dbus_message_get_args (message, &error,
+                                  DBUS_TYPE_STRING, &service_name,
+                                  DBUS_TYPE_INVALID))
+        {
+          if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
+            {
+              _dbus_verbose ("no memory to get service name arg\n");
+            }
+          else
+            {
+              _dbus_assert (dbus_error_is_set (&error));
+              _dbus_warn ("Did not get the expected single string argument\n");
+              goto out;
+            }
+        }
+      else if (strcmp (service_name, d->expected_service_name) != 0)
+        {
+          _dbus_warn ("expected creation of service %s, got creation of %s\n",
+                      d->expected_service_name,
+                      service_name);
+          goto out;
+        }
+    }
+
+  d->failed = FALSE;
+  
+ out:
+  dbus_free (service_name);
+  dbus_error_free (&error);
+  
+  if (message)
+    dbus_message_unref (message);
+
+  return !d->failed;
+}
+
+static dbus_bool_t
+check_no_leftovers (BusContext *context)
+{
+  CheckNoMessagesData nmd;
+
+  nmd.failed = FALSE;
+  bus_test_clients_foreach (check_no_messages_foreach,
+                            &nmd);
+  
+  if (nmd.failed)
+    return FALSE;
+  else
+    return TRUE;
 }
 
 /* returns TRUE if the correct thing happens,
@@ -408,8 +663,12 @@ check_hello_message (BusContext     *context,
   dbus_int32_t serial;
   dbus_bool_t retval;
   DBusError error;
-
+  char *name;
+  char *acquired;
+  
   dbus_error_init (&error);
+  name = NULL;
+  acquired = NULL;
   
   message = dbus_message_new (DBUS_SERVICE_DBUS,
 			      DBUS_MESSAGE_HELLO);
@@ -460,7 +719,7 @@ check_hello_message (BusContext     *context,
     }
   else
     {
-      char *str;
+      CheckServiceCreatedData scd;
       
       if (dbus_message_name_is (message,
                                 DBUS_MESSAGE_HELLO))
@@ -474,21 +733,91 @@ check_hello_message (BusContext     *context,
           goto out;
         }
 
+    retry_get_hello_name:
       if (!dbus_message_get_args (message, &error,
-                                  DBUS_TYPE_STRING, &str,
+                                  DBUS_TYPE_STRING, &name,
                                   DBUS_TYPE_INVALID))
         {
-          _dbus_warn ("Did not get the expected single string argument\n");
+          if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
+            {
+              _dbus_verbose ("no memory to get service name arg from hello\n");
+              dbus_error_free (&error);
+              bus_wait_for_memory ();
+              goto retry_get_hello_name;
+            }
+          else
+            {
+              _dbus_assert (dbus_error_is_set (&error));
+              _dbus_warn ("Did not get the expected single string argument to hello\n");
+              goto out;
+            }
+        }
+
+      _dbus_verbose ("Got hello name: %s\n", name);
+
+      while (!dbus_bus_set_base_service (connection, name))
+        bus_wait_for_memory ();
+      
+      scd.skip_connection = NULL;
+      scd.failed = FALSE;
+      scd.expected_service_name = name;
+      bus_test_clients_foreach (check_service_created_foreach,
+                                &scd);
+      
+      if (scd.failed)
+        goto out;
+      
+      /* Client should also have gotten ServiceAcquired */
+      dbus_message_unref (message);
+      message = dbus_connection_pop_message (connection);
+      if (message == NULL)
+        {
+          _dbus_warn ("Expecting %s, got nothing\n",
+                      DBUS_MESSAGE_SERVICE_ACQUIRED);
           goto out;
         }
+      
+    retry_get_acquired_name:
+      if (!dbus_message_get_args (message, &error,
+                                  DBUS_TYPE_STRING, &acquired,
+                                  DBUS_TYPE_INVALID))
+        {
+          if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
+            {
+              _dbus_verbose ("no memory to get service name arg from acquired\n");
+              dbus_error_free (&error);
+              bus_wait_for_memory ();
+              goto retry_get_acquired_name;
+            }
+          else
+            {
+              _dbus_assert (dbus_error_is_set (&error));
+              _dbus_warn ("Did not get the expected single string argument to ServiceAcquired\n");
+              goto out;
+            }
+        }
+
+      _dbus_verbose ("Got acquired name: %s\n", acquired);
 
-      _dbus_verbose ("Got hello name: %s\n", str);
-      dbus_free (str);
+      if (strcmp (acquired, name) != 0)
+        {
+          _dbus_warn ("Acquired name is %s but expected %s\n",
+                      acquired, name);
+          goto out;
+        }
     }
-      
+
+  if (!check_no_leftovers (context))
+    goto out;
+  
   retval = TRUE;
   
  out:
+  dbus_error_free (&error);
+  
+  dbus_free (name);
+  dbus_free (acquired);
+  
   if (message)
     dbus_message_unref (message);
   
@@ -522,7 +851,24 @@ check_hello_connection (BusContext *context)
   if (!check_hello_message (context, connection))
     return FALSE;
 
-  kill_client_connection (connection);
+  if (dbus_bus_get_base_service (connection) == NULL)
+    {
+      /* We didn't successfully register, so we can't
+       * do the usual kill_client_connection() checks
+       */
+      dbus_connection_ref (connection);
+      dbus_connection_disconnect (connection);
+      /* dispatching disconnect handler will unref once */
+      if (dbus_connection_dispatch_message (connection))
+        _dbus_assert_not_reached ("message other than disconnect dispatched after failure to register");
+      dbus_connection_unref (connection);
+      _dbus_assert (!bus_test_client_listed (connection));
+      return TRUE;
+    }
+  else
+    {
+      kill_client_connection (context, connection);
+    }
 
   return TRUE;
 }
@@ -560,6 +906,9 @@ check1_try_iterations (BusContext *context,
       if (! (*func) (context))
         _dbus_assert_not_reached ("test failed");
 
+      if (!check_no_leftovers (context))
+        _dbus_assert_not_reached ("Messages were left over, should be covered by test suite");
+      
       approx_mallocs -= 1;
     }
 
@@ -588,23 +937,29 @@ bus_dispatch_test (const DBusString *test_data_dir)
   if (foo == NULL)
     _dbus_assert_not_reached ("could not alloc connection");
 
+  if (!bus_setup_debug_client (foo))
+    _dbus_assert_not_reached ("could not set up connection");
+
+  if (!check_hello_message (context, foo))
+    _dbus_assert_not_reached ("hello message failed");
+  
   bar = dbus_connection_open ("debug-pipe:name=test-server", &result);
   if (bar == NULL)
     _dbus_assert_not_reached ("could not alloc connection");
 
+  if (!bus_setup_debug_client (bar))
+    _dbus_assert_not_reached ("could not set up connection");
+
+  if (!check_hello_message (context, bar))
+    _dbus_assert_not_reached ("hello message failed");
+  
   baz = dbus_connection_open ("debug-pipe:name=test-server", &result);
   if (baz == NULL)
     _dbus_assert_not_reached ("could not alloc connection");
 
-  if (!bus_setup_debug_client (foo) ||
-      !bus_setup_debug_client (bar) ||
-      !bus_setup_debug_client (baz))
+  if (!bus_setup_debug_client (baz))
     _dbus_assert_not_reached ("could not set up connection");
-  
-  if (!check_hello_message (context, foo))
-    _dbus_assert_not_reached ("hello message failed");
-  if (!check_hello_message (context, bar))
-    _dbus_assert_not_reached ("hello message failed");
+
   if (!check_hello_message (context, baz))
     _dbus_assert_not_reached ("hello message failed");
 
@@ -612,11 +967,24 @@ bus_dispatch_test (const DBusString *test_data_dir)
                          check_hello_connection);
 
   dbus_connection_disconnect (foo);
+  if (dbus_connection_dispatch_message (foo))
+    _dbus_assert_not_reached ("extra message in queue");
   dbus_connection_unref (foo);
+  _dbus_assert (!bus_test_client_listed (foo));
+
   dbus_connection_disconnect (bar);
+  if (dbus_connection_dispatch_message (bar))
+    _dbus_assert_not_reached ("extra message in queue");
   dbus_connection_unref (bar);
+  _dbus_assert (!bus_test_client_listed (bar));
+
   dbus_connection_disconnect (baz);
+  if (dbus_connection_dispatch_message (baz))
+    _dbus_assert_not_reached ("extra message in queue");
   dbus_connection_unref (baz);
+  _dbus_assert (!bus_test_client_listed (baz));
+
+  bus_context_unref (context);
   
   return TRUE;
 }
diff --git a/bus/test.c b/bus/test.c
index fe4cdc42..09e40f34 100644
--- a/bus/test.c
+++ b/bus/test.c
@@ -27,11 +27,13 @@
 #include "test.h"
 #include "loop.h"
 #include <dbus/dbus-internals.h>
+#include <dbus/dbus-list.h>
 
 /* The "debug client" watch/timeout handlers don't dispatch messages,
  * as we manually pull them in order to verify them. This is why they
  * are different from the real handlers in connection.c
  */
+static DBusList *clients = NULL;
 
 static void
 client_watch_callback (DBusWatch     *watch,
@@ -95,17 +97,29 @@ client_disconnect_handler (DBusMessageHandler *handler,
                            DBusMessage        *message,
                            void               *user_data)
 {
+  _dbus_verbose ("Removing client %p in disconnect handler\n",
+                 connection);
+  
+  _dbus_list_remove (&clients, connection);
+  
   dbus_connection_unref (connection);
   
   return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS;
 }
 
+static int handler_slot = -1;
+
 dbus_bool_t
 bus_setup_debug_client (DBusConnection *connection)
 {
   DBusMessageHandler *disconnect_handler;
   const char *to_handle[] = { DBUS_MESSAGE_LOCAL_DISCONNECT };
   dbus_bool_t retval;
+
+  if (handler_slot < 0)
+    handler_slot = dbus_connection_allocate_data_slot ();
+  if (handler_slot < 0)
+    return FALSE;
   
   disconnect_handler = dbus_message_handler_new (client_disconnect_handler,
                                                  NULL, NULL);
@@ -139,24 +153,71 @@ bus_setup_debug_client (DBusConnection *connection)
                                               connection, NULL))
     goto out;
 
+  if (!_dbus_list_append (&clients, connection))
+    goto out;
+
+  /* Set up handler to be destroyed */
+  if (!dbus_connection_set_data (connection, handler_slot,
+                                 disconnect_handler,
+                                 (DBusFreeFunction)
+                                 dbus_message_handler_unref))
+    goto out;
+  
   retval = TRUE;
   
  out:
   if (!retval)
     {
-      dbus_connection_unregister_handler (connection,
-                                          disconnect_handler,
-                                          to_handle,
-                                          _DBUS_N_ELEMENTS (to_handle));
+      dbus_message_handler_unref (disconnect_handler); /* unregisters it */
       
       dbus_connection_set_watch_functions (connection,
                                            NULL, NULL, NULL, NULL, NULL);
       dbus_connection_set_timeout_functions (connection,
                                              NULL, NULL, NULL, NULL, NULL);
+
+      _dbus_list_remove_last (&clients, connection);
     }
+      
+  return retval;
+}
+
+void
+bus_test_clients_foreach (BusConnectionForeachFunction  function,
+                          void                         *data)
+{
+  DBusList *link;
   
-  dbus_message_handler_unref (disconnect_handler);
+  link = _dbus_list_get_first_link (&clients);
+  while (link != NULL)
+    {
+      DBusConnection *connection = link->data;
+      DBusList *next = _dbus_list_get_next_link (&clients, link);
+
+      if (!(* function) (connection, data))
+        break;
+      
+      link = next;
+    }
+}
+
+dbus_bool_t
+bus_test_client_listed (DBusConnection *connection)
+{
+  DBusList *link;
   
-  return retval;
+  link = _dbus_list_get_first_link (&clients);
+  while (link != NULL)
+    {
+      DBusConnection *c = link->data;
+      DBusList *next = _dbus_list_get_next_link (&clients, link);
+
+      if (c == connection)
+        return TRUE;
+      
+      link = next;
+    }
+
+  return FALSE;
 }
+
 #endif
diff --git a/bus/test.h b/bus/test.h
index 6f76abf0..1cd580bf 100644
--- a/bus/test.h
+++ b/bus/test.h
@@ -30,10 +30,13 @@
 
 #include <dbus/dbus.h>
 #include <dbus/dbus-string.h>
+#include "connection.h"
 
-dbus_bool_t bus_dispatch_test (const DBusString *test_data_dir);
-
-dbus_bool_t bus_setup_debug_client  (DBusConnection *connection);
+dbus_bool_t bus_dispatch_test        (const DBusString             *test_data_dir);
+dbus_bool_t bus_setup_debug_client   (DBusConnection               *connection);
+void        bus_test_clients_foreach (BusConnectionForeachFunction  function,
+                                      void                         *data);
+dbus_bool_t bus_test_client_listed   (DBusConnection               *connection);
 
 #endif