2003-04-10 Havoc Pennington <hp@redhat.com>

	* dbus/dbus-connection.c (dbus_connection_flush): don't spin on
	the connection if it's disconnected

	* bus/activation.c (bus_activation_service_created): use new
	transaction features to roll back removal of pending activation if
	we don't successfully create the service after all. Don't remove
	pending activation if the function fails.

	* dbus/dbus-list.c (_dbus_list_insert_before_link)
	(_dbus_list_insert_after_link): new code to facilitate
	services.c fixes

	* dbus/dbus-hash.c (_dbus_hash_table_insert_string_preallocated):
	new functionality, so we can preallocate the ability to insert
	into a hash table.

	* bus/connection.c (bus_transaction_add_cancel_hook): new function
	allowing us to put custom hooks in a transaction to be used for
	cancelling said transaction

	* doc/dbus-specification.sgml: add some discussion of secondary
	service owners, and disallow zero-length service names

	* bus/services.c (bus_registry_acquire_service): new function,
	splits out part of bus_driver_handle_acquire_service() and fixes
	a bug where we didn't remove the service doing the acquiring
	from the secondary queue if we failed to remove the current owner
	from the front of the queue.

7 files changed, 731 insertions, 200 deletions

diff --git a/bus/activation.c b/bus/activation.c
index 13c147ea..64e0d914 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -63,6 +63,7 @@ struct BusPendingActivationEntry
 
 typedef struct
 {
+  int refcount;
   BusActivation *activation;
   char *service_name;
   DBusList *entries;
@@ -94,13 +95,26 @@ handle_timeout_callback (DBusTimeout   *timeout,
 }
 
 static void
-bus_pending_activation_free (BusPendingActivation *pending_activation)
+bus_pending_activation_ref (BusPendingActivation *pending_activation)
+{
+  _dbus_assert (pending_activation->refcount > 0);
+  pending_activation->refcount += 1;
+}
+
+static void
+bus_pending_activation_unref (BusPendingActivation *pending_activation)
 {
   DBusList *link;
   
   if (pending_activation == NULL) /* hash table requires this */
     return;
 
+  _dbus_assert (pending_activation->refcount > 0);
+  pending_activation->refcount -= 1;
+
+  if (pending_activation->refcount > 0)
+    return;
+  
   if (pending_activation->timeout_added)
     {
       _dbus_loop_remove_timeout (bus_context_get_loop (pending_activation->activation->context),
@@ -396,7 +410,7 @@ bus_activation_new (BusContext        *context,
     }
 
   activation->pending_activations = _dbus_hash_table_new (DBUS_HASH_STRING, NULL,
-							  (DBusFreeFunction)bus_pending_activation_free);
+							  (DBusFreeFunction)bus_pending_activation_unref);
 
   if (activation->pending_activations == NULL)
     {
@@ -466,6 +480,75 @@ child_setup (void *data)
     }
 }
 
+typedef struct
+{
+  BusPendingActivation *pending_activation;
+  DBusPreallocatedHash *hash_entry;
+} RestorePendingData;
+
+static void
+restore_pending (void *data)
+{
+  RestorePendingData *d = data;
+
+  _dbus_assert (d->pending_activation != NULL);
+  _dbus_assert (d->hash_entry != NULL);
+
+  _dbus_verbose ("Restoring pending activation for service %s, has timeout = %d\n",
+                 d->pending_activation->service_name,
+                 d->pending_activation->timeout_added);
+  
+  _dbus_hash_table_insert_string_preallocated (d->pending_activation->activation->pending_activations,
+                                               d->hash_entry,
+                                               d->pending_activation->service_name, d->pending_activation);
+
+  bus_pending_activation_ref (d->pending_activation);
+  
+  d->hash_entry = NULL;
+}
+
+static void
+free_pending_restore_data (void *data)
+{
+  RestorePendingData *d = data;
+
+  if (d->hash_entry)
+    _dbus_hash_table_free_preallocated_entry (d->pending_activation->activation->pending_activations,
+                                              d->hash_entry);
+
+  bus_pending_activation_unref (d->pending_activation);
+  
+  dbus_free (d);
+}
+
+static dbus_bool_t
+add_restore_pending_to_transaction (BusTransaction       *transaction,
+                                    BusPendingActivation *pending_activation)
+{
+  RestorePendingData *d;
+
+  d = dbus_new (RestorePendingData, 1);
+  if (d == NULL)
+    return FALSE;
+  
+  d->pending_activation = pending_activation;
+  d->hash_entry = _dbus_hash_table_preallocate_entry (d->pending_activation->activation->pending_activations);
+  
+  bus_pending_activation_ref (d->pending_activation);
+  
+  if (d->hash_entry == NULL ||
+      !bus_transaction_add_cancel_hook (transaction, restore_pending, d,
+                                        free_pending_restore_data))
+    {
+      free_pending_restore_data (d);
+      return FALSE;
+    }
+
+  _dbus_verbose ("Saved pending activation to be restored if the transaction fails\n");
+  
+  return TRUE;
+}
+
 dbus_bool_t
 bus_activation_service_created (BusActivation  *activation,
 				const char     *service_name,
@@ -521,13 +604,19 @@ bus_activation_service_created (BusActivation  *activation,
 
       link = next;
     }
+
+  if (!add_restore_pending_to_transaction (transaction, pending_activation))
+    {
+      _dbus_verbose ("Could not add cancel hook to transaction to revert removing pending activation\n");
+      BUS_SET_OOM (error);
+      goto error;
+    }
   
   _dbus_hash_table_remove_string (activation->pending_activations, service_name);
 
   return TRUE;
 
  error:
-  _dbus_hash_table_remove_string (activation->pending_activations, service_name);
   return FALSE;
 }
 
@@ -785,12 +874,13 @@ bus_activation_activate_service (BusActivation  *activation,
 	}
 
       pending_activation->activation = activation;
+      pending_activation->refcount = 1;
       
       pending_activation->service_name = _dbus_strdup (service_name);
       if (!pending_activation->service_name)
 	{
 	  BUS_SET_OOM (error);
-	  bus_pending_activation_free (pending_activation);
+	  bus_pending_activation_unref (pending_activation);
 	  bus_pending_activation_entry_free (pending_activation_entry);	  
 	  return FALSE;
 	}
@@ -803,7 +893,7 @@ bus_activation_activate_service (BusActivation  *activation,
       if (!pending_activation->timeout)
 	{
 	  BUS_SET_OOM (error);
-	  bus_pending_activation_free (pending_activation);
+	  bus_pending_activation_unref (pending_activation);
 	  bus_pending_activation_entry_free (pending_activation_entry);	  
 	  return FALSE;
 	}
@@ -815,7 +905,7 @@ bus_activation_activate_service (BusActivation  *activation,
                                    NULL))
 	{
 	  BUS_SET_OOM (error);
-	  bus_pending_activation_free (pending_activation);
+	  bus_pending_activation_unref (pending_activation);
 	  bus_pending_activation_entry_free (pending_activation_entry);	  
 	  return FALSE;
 	}
@@ -825,7 +915,7 @@ bus_activation_activate_service (BusActivation  *activation,
       if (!_dbus_list_append (&pending_activation->entries, pending_activation_entry))
 	{
 	  BUS_SET_OOM (error);
-	  bus_pending_activation_free (pending_activation);
+	  bus_pending_activation_unref (pending_activation);
 	  bus_pending_activation_entry_free (pending_activation_entry);	  
 	  return FALSE;
 	}
@@ -834,7 +924,7 @@ bus_activation_activate_service (BusActivation  *activation,
 					   pending_activation->service_name, pending_activation))
 	{
 	  BUS_SET_OOM (error);
-	  bus_pending_activation_free (pending_activation);
+	  bus_pending_activation_unref (pending_activation);
 	  return FALSE;
 	}
     }
diff --git a/bus/connection.c b/bus/connection.c
index 80a9ae7a..2cfbeb27 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -139,6 +139,8 @@ bus_connection_disconnected (DBusConnection *connection)
       if (!bus_service_remove_owner (service, connection,
                                      transaction, &error))
         {
+          _DBUS_ASSERT_ERROR_IS_SET (&error);
+          
           if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
             {
               dbus_error_free (&error);
@@ -147,7 +149,11 @@ bus_connection_disconnected (DBusConnection *connection)
               goto retry;
             }
           else
-            _dbus_assert_not_reached ("Removing service owner failed for non-memory-related reason");
+            {
+              _dbus_verbose ("Failed to remove service owner: %s %s\n",
+                             error.name, error.message);
+              _dbus_assert_not_reached ("Removing service owner failed for non-memory-related reason");
+            }
         }
         
       bus_transaction_execute_and_free (transaction);
@@ -746,19 +752,31 @@ bus_connection_send_oom_error (DBusConnection *connection,
   d->oom_preallocated = NULL;
 }
 
-dbus_bool_t
-bus_connection_add_owned_service (DBusConnection *connection,
-                                  BusService     *service)
+void
+bus_connection_add_owned_service_link (DBusConnection *connection,
+                                       DBusList       *link)
 {
   BusConnectionData *d;
 
   d = BUS_CONNECTION_DATA (connection);
   _dbus_assert (d != NULL);
 
-  if (!_dbus_list_append (&d->services_owned,
-                          service))
+  _dbus_list_append_link (&d->services_owned, link);
+}
+
+dbus_bool_t
+bus_connection_add_owned_service (DBusConnection *connection,
+                                  BusService     *service)
+{
+  DBusList *link;
+
+  link = _dbus_list_alloc_link (service);
+
+  if (link == NULL)
     return FALSE;
 
+  bus_connection_add_owned_service_link (connection, link);
+
   return TRUE;
 }
 
@@ -805,6 +823,13 @@ bus_connection_get_name (DBusConnection *connection)
   return d->name;
 }
 
+/**
+ * Transactions
+ *
+ * Note that this is fairly fragile; in particular, don't try to use
+ * one transaction across any main loop iterations.
+ */
+
 typedef struct
 {
   BusTransaction *transaction;
@@ -812,10 +837,18 @@ typedef struct
   DBusPreallocatedSend *preallocated;
 } MessageToSend;
 
+typedef struct
+{
+  BusTransactionCancelFunction cancel_function;
+  DBusFreeFunction free_data_function;
+  void *data;
+} CancelHook;
+
 struct BusTransaction
 {
   DBusList *connections;
   BusContext *context;
+  DBusList *cancel_hooks;
 };
 
 static void
@@ -831,6 +864,39 @@ message_to_send_free (DBusConnection *connection,
   dbus_free (to_send);
 }
 
+static void
+cancel_hook_cancel (void *element,
+                    void *data)
+{
+  CancelHook *ch = element;
+
+  _dbus_verbose ("Running transaction cancel hook\n");
+  
+  if (ch->cancel_function)
+    (* ch->cancel_function) (ch->data);  
+}
+
+static void
+cancel_hook_free (void *element,
+                  void *data)
+{
+  CancelHook *ch = element;
+
+  if (ch->free_data_function)
+    (* ch->free_data_function) (ch->data);
+
+  dbus_free (ch);
+}
+
+static void
+free_cancel_hooks (BusTransaction *transaction)
+{
+  _dbus_list_foreach (&transaction->cancel_hooks,
+                      cancel_hook_free, NULL);
+  
+  _dbus_list_clear (&transaction->cancel_hooks);
+}
+
 BusTransaction*
 bus_transaction_new (BusContext *context)
 {
@@ -980,6 +1046,11 @@ bus_transaction_cancel_and_free (BusTransaction *transaction)
 
   _dbus_assert (transaction->connections == NULL);
 
+  _dbus_list_foreach (&transaction->cancel_hooks,
+                      cancel_hook_cancel, NULL);
+
+  free_cancel_hooks (transaction);
+  
   dbus_free (transaction);
 }
 
@@ -1036,6 +1107,8 @@ bus_transaction_execute_and_free (BusTransaction *transaction)
 
   _dbus_assert (transaction->connections == NULL);
 
+  free_cancel_hooks (transaction);
+  
   dbus_free (transaction);
 }
 
@@ -1090,3 +1163,31 @@ bus_transaction_send_error_reply (BusTransaction  *transaction,
   
   return TRUE;
 }
+
+dbus_bool_t
+bus_transaction_add_cancel_hook (BusTransaction               *transaction,
+                                 BusTransactionCancelFunction  cancel_function,
+                                 void                         *data,
+                                 DBusFreeFunction              free_data_function)
+{
+  CancelHook *ch;
+
+  ch = dbus_new (CancelHook, 1);
+  if (ch == NULL)
+    return FALSE;
+  
+  ch->cancel_function = cancel_function;
+  ch->data = data;
+  ch->free_data_function = free_data_function;
+
+  /* It's important that the hooks get run in reverse order that they
+   * were added
+   */
+  if (!_dbus_list_prepend (&transaction->cancel_hooks, ch))
+    {
+      dbus_free (ch);
+      return FALSE;
+    }
+
+  return TRUE;
+}
diff --git a/bus/connection.h b/bus/connection.h
index 0d64e987..6108bdfd 100644
--- a/bus/connection.h
+++ b/bus/connection.h
@@ -25,6 +25,7 @@
 #define BUS_CONNECTION_H
 
 #include <dbus/dbus.h>
+#include <dbus/dbus-list.h>
 #include "bus.h"
 
 typedef dbus_bool_t (* BusConnectionForeachFunction) (DBusConnection *connection, 
@@ -53,10 +54,13 @@ void        bus_connection_send_oom_error        (DBusConnection *connection,
                                                   DBusMessage    *in_reply_to);
 
 /* called by services.c */
-dbus_bool_t bus_connection_add_owned_service    (DBusConnection *connection,
-                                                 BusService     *service);
-void        bus_connection_remove_owned_service (DBusConnection *connection,
-                                                 BusService     *service);
+dbus_bool_t bus_connection_add_owned_service      (DBusConnection *connection,
+                                                   BusService     *service);
+void        bus_connection_remove_owned_service   (DBusConnection *connection,
+                                                   BusService     *service);
+void        bus_connection_add_owned_service_link (DBusConnection *connection,
+                                                   DBusList       *link);
+
 
 /* called by driver.c */
 dbus_bool_t bus_connection_set_name (DBusConnection               *connection,
@@ -74,18 +78,24 @@ dbus_bool_t bus_connection_get_groups  (DBusConnection       *connection,
 BusPolicy*  bus_connection_get_policy  (DBusConnection       *connection);
 
 /* transaction API so we can send or not send a block of messages as a whole */
-BusTransaction* bus_transaction_new              (BusContext      *context);
-BusContext*     bus_transaction_get_context      (BusTransaction  *transaction);
-BusConnections* bus_transaction_get_connections  (BusTransaction  *transaction);
-dbus_bool_t     bus_transaction_send_message     (BusTransaction  *transaction,
-                                                  DBusConnection  *connection,
-                                                  DBusMessage     *message);
-dbus_bool_t     bus_transaction_send_error_reply (BusTransaction  *transaction,
-                                                  DBusConnection  *connection,
-                                                  const DBusError *error,
-                                                  DBusMessage     *in_reply_to);
-void            bus_transaction_cancel_and_free  (BusTransaction  *transaction);
-void            bus_transaction_execute_and_free (BusTransaction  *transaction);
 
+typedef void (* BusTransactionCancelFunction) (void *data);
+
+BusTransaction* bus_transaction_new              (BusContext                   *context);
+BusContext*     bus_transaction_get_context      (BusTransaction               *transaction);
+BusConnections* bus_transaction_get_connections  (BusTransaction               *transaction);
+dbus_bool_t     bus_transaction_send_message     (BusTransaction               *transaction,
+                                                  DBusConnection               *connection,
+                                                  DBusMessage                  *message);
+dbus_bool_t     bus_transaction_send_error_reply (BusTransaction               *transaction,
+                                                  DBusConnection               *connection,
+                                                  const DBusError              *error,
+                                                  DBusMessage                  *in_reply_to);
+void            bus_transaction_cancel_and_free  (BusTransaction               *transaction);
+void            bus_transaction_execute_and_free (BusTransaction               *transaction);
+dbus_bool_t     bus_transaction_add_cancel_hook  (BusTransaction               *transaction,
+                                                  BusTransactionCancelFunction  cancel_function,
+                                                  void                         *data,
+                                                  DBusFreeFunction              free_data_function);
 
 #endif /* BUS_CONNECTION_H */
diff --git a/bus/dispatch.c b/bus/dispatch.c
index e867674b..f6ddc76a 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -1442,6 +1442,47 @@ check_send_exit_to_service (BusContext     *context,
   return retval;
 }
 
+static dbus_bool_t
+check_got_error (BusContext     *context,
+                 DBusConnection *connection,
+                 const char     *error_name)
+{
+  DBusMessage *message;
+  dbus_bool_t retval;
+
+  retval = FALSE;
+  
+  message = pop_message_waiting_for_memory (connection);
+  if (message == NULL)
+    {
+      _dbus_warn ("Did not get an expected error\n");
+      goto out;
+    }
+
+  if (!dbus_message_get_is_error (message))
+    {
+      _dbus_warn ("Expected an error, got %s\n",
+                  dbus_message_get_name (message));
+      goto out;
+    }
+
+  if (!dbus_message_name_is (message, error_name))
+    {
+      _dbus_warn ("Expected error %s, got %s instead\n",
+                  error_name,
+                  dbus_message_get_name (message));
+      goto out;
+    }
+
+  retval = TRUE;
+  
+ out:
+  if (message)
+    dbus_message_unref (message);
+  
+  return retval;
+}
+          
 #define EXISTENT_SERVICE_NAME "org.freedesktop.DBus.TestSuiteEchoService"
 
 /* returns TRUE if the correct thing happens,
@@ -1551,6 +1592,7 @@ check_existent_service_activation (BusContext     *context,
   else
     {
       dbus_bool_t got_service_deleted;
+      dbus_bool_t got_error;
       
       if (!check_base_service_activated (context, connection,
                                          message, &base_service))
@@ -1570,9 +1612,22 @@ check_existent_service_activation (BusContext     *context,
         }
 
       got_service_deleted = dbus_message_name_is (message, DBUS_MESSAGE_SERVICE_DELETED);
-
+      got_error = dbus_message_get_is_error (message);
+      
       dbus_connection_return_message (connection, message);
       message = NULL;
+
+      if (got_error)
+        {
+          if (!check_got_error (context, connection,
+                                DBUS_ERROR_SPAWN_CHILD_EXITED))
+            goto out;
+
+          /* A service deleted should be coming along now after this error.
+           * We can also get the error *after* the service deleted.
+           */
+          got_service_deleted = TRUE;
+        }
       
       if (got_service_deleted)
         {
@@ -1589,34 +1644,19 @@ check_existent_service_activation (BusContext     *context,
           if (csdd.failed)
             goto out;
 
-          /* Now we should get an error about the service exiting */
-          block_connection_until_message_from_bus (context, connection);
-          
-          /* and process everything again */
-          bus_test_run_everything (context);
-          
-          message = pop_message_waiting_for_memory (connection);
-          if (message == NULL)
-            {
-              _dbus_warn ("Did not get an error from the service %s exiting\n",
-                          EXISTENT_SERVICE_NAME);
-              goto out;
-            }
-
-          if (!dbus_message_get_is_error (message))
-            {
-              _dbus_warn ("Expected an error due to service exiting, got %s\n",
-                          dbus_message_get_name (message));
-              goto out;
-            }
-
-          if (!dbus_message_name_is (message,
-                                     DBUS_ERROR_SPAWN_CHILD_EXITED))
+          /* Now we should get an error about the service exiting
+           * if we didn't get it before.
+           */
+          if (!got_error)
             {
-              _dbus_warn ("Expected error %s on service exit, got %s instead\n",
-                          DBUS_ERROR_SPAWN_CHILD_EXITED,
-                          dbus_message_get_name (message));
-              goto out;
+              block_connection_until_message_from_bus (context, connection);
+              
+              /* and process everything again */
+              bus_test_run_everything (context);
+              
+              if (!check_got_error (context, connection,
+                                    DBUS_ERROR_SPAWN_CHILD_EXITED))
+                goto out;
             }
         }
       else
@@ -1785,7 +1825,7 @@ bus_dispatch_test (const DBusString *test_data_dir)
   if (!check_hello_message (context, baz))
     _dbus_assert_not_reached ("hello message failed");
 
-#if 0
+#if 1
   check2_try_iterations (context, foo, "existent_service_activation",
                          check_existent_service_activation);
 #endif
diff --git a/bus/driver.c b/bus/driver.c
index bb8ac296..33017e9f 100644
--- a/bus/driver.c
+++ b/bus/driver.c
@@ -438,13 +438,10 @@ bus_driver_handle_acquire_service (DBusConnection *connection,
 {
   DBusMessage *reply;
   DBusString service_name;
-  BusService *service;  
   char *name;
   int service_reply;
   int flags;
   dbus_bool_t retval;
-  DBusConnection *old_owner;
-  DBusConnection *current_owner;
   BusRegistry *registry;
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
@@ -461,27 +458,14 @@ bus_driver_handle_acquire_service (DBusConnection *connection,
   
   retval = FALSE;
   reply = NULL;
-  
-  if (*name == ':')
-    {
-      /* Not allowed; only base services can start with ':' */
-      dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
-                      "Cannot acquire a service starting with ':' such as \"%s\"",
-                      name);
-
-      _dbus_verbose ("Attempt to acquire invalid base service name \"%s\"", name);
-      
-      goto out;
-    }
 
   _dbus_string_init_const (&service_name, name);
-  
-  service = bus_registry_lookup (registry, &service_name);
 
-  if (service != NULL)
-    old_owner = bus_service_get_primary_owner (service);
-  else
-    old_owner = NULL;  
+  if (!bus_registry_acquire_service (registry, connection,
+                                     &service_name, flags,
+                                     &service_reply, transaction,
+                                     error))
+    goto out;
   
   reply = dbus_message_new_reply (message);
   if (reply == NULL)
@@ -495,67 +479,8 @@ bus_driver_handle_acquire_service (DBusConnection *connection,
       BUS_SET_OOM (error);
       goto out;
     }
-      
-  if (service == NULL)
-    {
-      service = bus_registry_ensure (registry,
-                                     &service_name, connection, transaction, error);
-      if (service == NULL)
-        goto out;
-    }
-
-  current_owner = bus_service_get_primary_owner (service);
-
-  if (old_owner == NULL)
-    {
-      _dbus_assert (current_owner == connection);
-
-      bus_service_set_prohibit_replacement (service,
-					    (flags & DBUS_SERVICE_FLAG_PROHIBIT_REPLACEMENT));      
-			
-      service_reply = DBUS_SERVICE_REPLY_PRIMARY_OWNER;      
-    }
-  else if (old_owner == connection)
-    service_reply = DBUS_SERVICE_REPLY_ALREADY_OWNER;
-  else if (!((flags & DBUS_SERVICE_FLAG_REPLACE_EXISTING)))
-    service_reply = DBUS_SERVICE_REPLY_SERVICE_EXISTS;
-  else if (bus_service_get_prohibit_replacement (service))
-    {
-      /* Queue the connection */
-      if (!bus_service_add_owner (service, connection,
-                                  transaction, error))
-        goto out;
-      
-      service_reply = DBUS_SERVICE_REPLY_IN_QUEUE;
-    }
-  else
-    {
-      /* Replace the current owner */
-
-      /* We enqueue the new owner and remove the first one because
-       * that will cause ServiceAcquired and ServiceLost messages to
-       * be sent.
-       */
-      
-      /* FIXME this is broken, if the remove_owner fails
-       * we don't undo the add_owner
-       * (easiest fix is probably to move all this to
-       * services.c and have a single routine for it)
-       */
-      
-      if (!bus_service_add_owner (service, connection,
-                                  transaction, error))
-        goto out;
-      
-      if (!bus_service_remove_owner (service, old_owner,
-                                     transaction, error))
-        goto out;
-      
-      _dbus_assert (connection == bus_service_get_primary_owner (service));
-      service_reply = DBUS_SERVICE_REPLY_PRIMARY_OWNER;
-    }
 
-  if (!dbus_message_append_args (reply, DBUS_TYPE_UINT32, service_reply, 0))
+  if (!dbus_message_append_args (reply, DBUS_TYPE_UINT32, service_reply, DBUS_TYPE_INVALID))
     {
       BUS_SET_OOM (error);
       goto out;
diff --git a/bus/services.c b/bus/services.c
index 3a7a0756..dfc3ed08 100644
--- a/bus/services.c
+++ b/bus/services.c
@@ -33,6 +33,8 @@
 
 struct BusService
 {
+  int refcount;
+
   BusRegistry *registry;
   char *name;
   DBusList *owners;
@@ -142,7 +144,8 @@ bus_registry_ensure (BusRegistry               *registry,
     }
 
   service->registry = registry;  
-
+  service->refcount = 1;
+  
   if (!_dbus_string_copy_data (service_name, &service->name))
     {
       _dbus_mem_pool_dealloc (registry->service_pool, service);
@@ -152,24 +155,21 @@ bus_registry_ensure (BusRegistry               *registry,
 
   if (!bus_driver_send_service_created (service->name, transaction, error))
     {
-      dbus_free (service->name);
-      _dbus_mem_pool_dealloc (registry->service_pool, service);
+      bus_service_unref (service);
       return NULL;
     }
 
   if (!bus_activation_service_created (bus_context_get_activation (registry->context),
 				       service->name, transaction, error))
     {
-      dbus_free (service->name);
-      _dbus_mem_pool_dealloc (registry->service_pool, service);
+      bus_service_unref (service);
       return NULL;
     }
   
   if (!bus_service_add_owner (service, owner_if_created,
                               transaction, error))
     {
-      dbus_free (service->name);
-      _dbus_mem_pool_dealloc (registry->service_pool, service);
+      bus_service_unref (service);
       return NULL;
     }
   
@@ -177,11 +177,7 @@ bus_registry_ensure (BusRegistry               *registry,
                                        service->name,
                                        service))
     {
-      bus_connection_remove_owned_service (owner_if_created,
-                                           service);
-      _dbus_list_clear (&service->owners);
-      dbus_free (service->name);
-      _dbus_mem_pool_dealloc (registry->service_pool, service);
+      /* The add_owner gets reverted on transaction cancel */
       BUS_SET_OOM (error);
       return NULL;
     }
@@ -250,6 +246,209 @@ bus_registry_list_services (BusRegistry *registry,
 }
 
 dbus_bool_t
+bus_registry_acquire_service (BusRegistry      *registry,
+                              DBusConnection   *connection,
+                              const DBusString *service_name,
+                              dbus_uint32_t     flags,
+                              dbus_uint32_t    *result,
+                              BusTransaction   *transaction,
+                              DBusError        *error)
+{
+  dbus_bool_t retval;
+  DBusConnection *old_owner;
+  DBusConnection *current_owner;
+  BusService *service;
+  
+  retval = FALSE;
+
+  if (_dbus_string_get_length (service_name) == 0)
+    {
+      dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
+                      "Zero-length service name is not allowed");
+      
+      _dbus_verbose ("Attempt to acquire zero-length service name\n");
+      
+      goto out;
+    }
+  
+  if (_dbus_string_get_byte (service_name, 0) == ':')
+    {
+      /* Not allowed; only base services can start with ':' */
+      dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
+                      "Cannot acquire a service starting with ':' such as \"%s\"",
+                      _dbus_string_get_const_data (service_name));
+      
+      _dbus_verbose ("Attempt to acquire invalid base service name \"%s\"",
+                     _dbus_string_get_const_data (service_name));
+      
+      goto out;
+    }
+  
+  service = bus_registry_lookup (registry, service_name);
+
+  if (service != NULL)
+    old_owner = bus_service_get_primary_owner (service);
+  else
+    old_owner = NULL;
+      
+  if (service == NULL)
+    {
+      service = bus_registry_ensure (registry,
+                                     service_name, connection, transaction, error);
+      if (service == NULL)
+        goto out;
+    }
+
+  current_owner = bus_service_get_primary_owner (service);
+
+  if (old_owner == NULL)
+    {
+      _dbus_assert (current_owner == connection);
+
+      bus_service_set_prohibit_replacement (service,
+					    (flags & DBUS_SERVICE_FLAG_PROHIBIT_REPLACEMENT));      
+			
+      *result = DBUS_SERVICE_REPLY_PRIMARY_OWNER;      
+    }
+  else if (old_owner == connection)
+    *result = DBUS_SERVICE_REPLY_ALREADY_OWNER;
+  else if (!((flags & DBUS_SERVICE_FLAG_REPLACE_EXISTING)))
+    *result = DBUS_SERVICE_REPLY_SERVICE_EXISTS;
+  else if (bus_service_get_prohibit_replacement (service))
+    {
+      /* Queue the connection */
+      if (!bus_service_add_owner (service, connection,
+                                  transaction, error))
+        goto out;
+      
+      *result = DBUS_SERVICE_REPLY_IN_QUEUE;
+    }
+  else
+    {
+      /* Replace the current owner */
+
+      /* We enqueue the new owner and remove the first one because
+       * that will cause ServiceAcquired and ServiceLost messages to
+       * be sent.
+       */
+      
+      if (!bus_service_add_owner (service, connection,
+                                  transaction, error))
+        goto out;
+
+      if (!bus_service_remove_owner (service, old_owner,
+                                     transaction, error))
+        goto out;
+      
+      _dbus_assert (connection == bus_service_get_primary_owner (service));
+      *result = DBUS_SERVICE_REPLY_PRIMARY_OWNER;
+    }
+
+  retval = TRUE;
+  
+ out:
+  return retval;
+}
+
+static void
+bus_service_unlink_owner (BusService      *service,
+                          DBusConnection  *owner)
+{
+  _dbus_list_remove_last (&service->owners, owner);
+  bus_connection_remove_owned_service (owner, service);
+}
+
+static void
+bus_service_unlink (BusService *service)
+{
+  _dbus_assert (service->owners == NULL);
+
+  /* the service may not be in the hash, if
+   * the failure causing transaction cancel
+   * was in the right place, but that's OK
+   */
+  _dbus_hash_table_remove_string (service->registry->service_hash,
+                                  service->name);
+  
+  bus_service_unref (service);
+}
+
+static void
+bus_service_relink (BusService           *service,
+                    DBusPreallocatedHash *preallocated)
+{
+  _dbus_assert (service->owners == NULL);
+  _dbus_assert (preallocated != NULL);
+
+  _dbus_hash_table_insert_string_preallocated (service->registry->service_hash,
+                                               preallocated,
+                                               service->name,
+                                               service);
+  
+  bus_service_ref (service);
+}
+
+typedef struct
+{
+  DBusConnection *connection;
+  BusService *service;
+} OwnershipCancelData;
+
+static void
+cancel_ownership (void *data)
+{
+  OwnershipCancelData *d = data;
+
+  /* We don't need to send messages notifying of these
+   * changes, since we're reverting something that was
+   * cancelled (effectively never really happened)
+   */
+  bus_service_unlink_owner (d->service, d->connection);
+  
+  if (d->service->owners == NULL)
+    bus_service_unlink (d->service);
+}
+
+static void
+free_ownership_cancel_data (void *data)
+{
+  OwnershipCancelData *d = data;
+
+  dbus_connection_unref (d->connection);
+  bus_service_unref (d->service);
+  
+  dbus_free (d);
+}
+
+static dbus_bool_t
+add_cancel_ownership_to_transaction (BusTransaction *transaction,
+                                     BusService     *service,
+                                     DBusConnection *connection)
+{
+  OwnershipCancelData *d;
+
+  d = dbus_new (OwnershipCancelData, 1);
+  if (d == NULL)
+    return FALSE;
+  
+  d->service = service;
+  d->connection = connection;
+
+  if (!bus_transaction_add_cancel_hook (transaction, cancel_ownership, d,
+                                        free_ownership_cancel_data))
+    {
+      dbus_free (d);
+      return FALSE;
+    }
+
+  bus_service_ref (d->service);
+  dbus_connection_ref (d->connection);
+  
+  return TRUE;
+}
+
+/* this function is self-cancelling if you cancel the transaction */
+dbus_bool_t
 bus_service_add_owner (BusService     *service,
                        DBusConnection *owner,
                        BusTransaction *transaction,
@@ -279,10 +478,147 @@ bus_service_add_owner (BusService     *service,
       BUS_SET_OOM (error);
       return FALSE;
     }
+
+  if (!add_cancel_ownership_to_transaction (transaction,
+                                            service,
+                                            owner))
+    {
+      bus_service_unlink_owner (service, owner);
+      BUS_SET_OOM (error);
+      return FALSE;
+    }
   
   return TRUE;
 }
 
+typedef struct
+{
+  DBusConnection *connection;
+  BusService     *service;
+  DBusConnection *before_connection; /* restore to position before this connection in owners list */
+  DBusList       *connection_link;
+  DBusList       *service_link;
+  DBusPreallocatedHash *hash_entry;
+} OwnershipRestoreData;
+
+static void
+restore_ownership (void *data)
+{
+  OwnershipRestoreData *d = data;
+  DBusList *link;
+
+  _dbus_assert (d->service_link != NULL);
+  _dbus_assert (d->connection_link != NULL);
+  
+  if (d->service->owners == NULL)
+    {
+      _dbus_assert (d->hash_entry != NULL);
+      bus_service_relink (d->service, d->hash_entry);
+    }
+  else
+    {
+      _dbus_assert (d->hash_entry == NULL);
+    }
+  
+  /* We don't need to send messages notifying of these
+   * changes, since we're reverting something that was
+   * cancelled (effectively never really happened)
+   */
+  link = _dbus_list_get_first_link (&d->service->owners);
+  while (link != NULL)
+    {
+      if (link->data == d->before_connection)
+        break;
+
+      link = _dbus_list_get_next_link (&d->service->owners, link);
+    }
+  
+  _dbus_list_insert_before_link (&d->service->owners, link, d->connection_link);
+
+  /* Note that removing then restoring this changes the order in which
+   * ServiceDeleted messages are sent on destruction of the
+   * connection.  This should be OK as the only guarantee there is
+   * that the base service is destroyed last, and we never even
+   * tentatively remove the base service.
+   */
+  bus_connection_add_owned_service_link (d->connection, d->service_link);
+  
+  d->hash_entry = NULL;
+  d->service_link = NULL;
+  d->connection_link = NULL;
+}
+
+static void
+free_ownership_restore_data (void *data)
+{
+  OwnershipRestoreData *d = data;
+
+  if (d->service_link)
+    _dbus_list_free_link (d->service_link);
+  if (d->connection_link)
+    _dbus_list_free_link (d->connection_link);
+  if (d->hash_entry)
+    _dbus_hash_table_free_preallocated_entry (d->service->registry->service_hash,
+                                              d->hash_entry);
+
+  dbus_connection_unref (d->connection);
+  bus_service_unref (d->service);
+  
+  dbus_free (d);
+}
+
+static dbus_bool_t
+add_restore_ownership_to_transaction (BusTransaction *transaction,
+                                      BusService     *service,
+                                      DBusConnection *connection)
+{
+  OwnershipRestoreData *d;
+  DBusList *link;
+
+  d = dbus_new (OwnershipRestoreData, 1);
+  if (d == NULL)
+    return FALSE;
+  
+  d->service = service;
+  d->connection = connection;
+  d->service_link = _dbus_list_alloc_link (service);
+  d->connection_link = _dbus_list_alloc_link (connection);
+  d->hash_entry = _dbus_hash_table_preallocate_entry (service->registry->service_hash);
+  
+  bus_service_ref (d->service);
+  dbus_connection_ref (d->connection);
+
+  d->before_connection = NULL;
+  link = _dbus_list_get_first_link (&service->owners);
+  while (link != NULL)
+    {
+      if (link->data == connection)
+        {
+          link = _dbus_list_get_next_link (&service->owners, link);
+
+          if (link)
+            d->before_connection = link->data;
+
+          break;
+        }
+      
+      link = _dbus_list_get_next_link (&service->owners, link);
+    }
+  
+  if (d->service_link == NULL ||
+      d->connection_link == NULL ||
+      d->hash_entry == NULL ||
+      !bus_transaction_add_cancel_hook (transaction, restore_ownership, d,
+                                        free_ownership_restore_data))
+    {
+      free_ownership_restore_data (d);
+      return FALSE;
+    }
+  
+  return TRUE;
+}
+
+/* this function is self-cancelling if you cancel the transaction */
 dbus_bool_t
 bus_service_remove_owner (BusService     *service,
                           DBusConnection *owner,
@@ -309,7 +645,6 @@ bus_service_remove_owner (BusService     *service,
     }
   else if (_dbus_list_length_is_one (&service->owners))
     {
-      /* We are the only owner - send service deleted */
       if (!bus_driver_send_service_deleted (service->name,
                                             transaction, error))
         return FALSE;
@@ -321,31 +656,52 @@ bus_service_remove_owner (BusService     *service,
       _dbus_assert (link != NULL);
       link = _dbus_list_get_next_link (&service->owners, link);
 
-      if (link != NULL)
-        {
-          /* This will be our new owner */
-          if (!bus_driver_send_service_acquired (link->data,
-                                                 service->name,
-                                                 transaction,
-                                                 error))
-            return FALSE;
-        }
+      _dbus_assert (link != NULL);
+
+      /* This will be our new owner */
+      if (!bus_driver_send_service_acquired (link->data,
+                                             service->name,
+                                             transaction,
+                                             error))
+        return FALSE;
+    }
+
+  if (!add_restore_ownership_to_transaction (transaction, service, owner))
+    {
+      BUS_SET_OOM (error);
+      return FALSE;
     }
   
-  _dbus_list_remove_last (&service->owners, owner);
-  bus_connection_remove_owned_service (owner, service);
+  bus_service_unlink_owner (service, owner);
 
   if (service->owners == NULL)
+    bus_service_unlink (service);
+
+  return TRUE;
+}
+
+void
+bus_service_ref (BusService *service)
+{
+  _dbus_assert (service->refcount > 0);
+  
+  service->refcount += 1;
+}
+
+void
+bus_service_unref (BusService *service)
+{
+  _dbus_assert (service->refcount > 0);
+  
+  service->refcount -= 1;
+
+  if (service->refcount == 0)
     {
-      /* Delete service (already sent message that it was deleted above) */
-      _dbus_hash_table_remove_string (service->registry->service_hash,
-                                      service->name);
+      _dbus_assert (service->owners == NULL);
       
       dbus_free (service->name);
       _dbus_mem_pool_dealloc (service->registry->service_pool, service);
     }
-
-  return TRUE;
 }
 
 DBusConnection*
diff --git a/bus/services.h b/bus/services.h
index aba2989a..bed950c3 100644
--- a/bus/services.h
+++ b/bus/services.h
@@ -32,38 +32,47 @@
 typedef void (* BusServiceForeachFunction) (BusService       *service,
                                             void             *data);
 
-BusRegistry* bus_registry_new           (BusContext                  *context);
-void         bus_registry_ref           (BusRegistry                 *registry);
-void         bus_registry_unref         (BusRegistry                 *registry);
-BusService*  bus_registry_lookup        (BusRegistry                 *registry,
-                                         const DBusString            *service_name);
-BusService*  bus_registry_ensure        (BusRegistry                 *registry,
-                                         const DBusString            *service_name,
-                                         DBusConnection              *owner_if_created,
-                                         BusTransaction              *transaction,
-                                         DBusError                   *error);
-void         bus_registry_foreach       (BusRegistry                 *registry,
-                                         BusServiceForeachFunction    function,
-                                         void                        *data);
-dbus_bool_t  bus_registry_list_services (BusRegistry                 *registry,
-                                         char                      ***listp,
-                                         int                         *array_len);
+BusRegistry* bus_registry_new             (BusContext                  *context);
+void         bus_registry_ref             (BusRegistry                 *registry);
+void         bus_registry_unref           (BusRegistry                 *registry);
+BusService*  bus_registry_lookup          (BusRegistry                 *registry,
+                                           const DBusString            *service_name);
+BusService*  bus_registry_ensure          (BusRegistry                 *registry,
+                                           const DBusString            *service_name,
+                                           DBusConnection              *owner_if_created,
+                                           BusTransaction              *transaction,
+                                           DBusError                   *error);
+void         bus_registry_foreach         (BusRegistry                 *registry,
+                                           BusServiceForeachFunction    function,
+                                           void                        *data);
+dbus_bool_t  bus_registry_list_services   (BusRegistry                 *registry,
+                                           char                      ***listp,
+                                           int                         *array_len);
+dbus_bool_t  bus_registry_acquire_service (BusRegistry                 *registry,
+                                           DBusConnection              *connection,
+                                           const DBusString            *service_name,
+                                           dbus_uint32_t                flags,
+                                           dbus_uint32_t               *result,
+                                           BusTransaction              *transaction,
+                                           DBusError                   *error);
 
+void            bus_service_ref                      (BusService     *service);
+void            bus_service_unref                    (BusService     *service);
+dbus_bool_t     bus_service_add_owner                (BusService     *service,
+                                                      DBusConnection *owner,
+                                                      BusTransaction *transaction,
+                                                      DBusError      *error);
+dbus_bool_t     bus_service_remove_owner             (BusService     *service,
+                                                      DBusConnection *owner,
+                                                      BusTransaction *transaction,
+                                                      DBusError      *error);
+dbus_bool_t     bus_service_has_owner                (BusService     *service,
+                                                      DBusConnection *owner);
+DBusConnection* bus_service_get_primary_owner        (BusService     *service);
+void            bus_service_set_prohibit_replacement (BusService     *service,
+                                                      dbus_bool_t     prohibit_replacement);
+dbus_bool_t     bus_service_get_prohibit_replacement (BusService     *service);
+const char*     bus_service_get_name                 (BusService     *service);
 
-dbus_bool_t     bus_service_add_owner                (BusService                *service,
-						      DBusConnection            *owner,
-                                                      BusTransaction            *transaction,
-                                                      DBusError                 *error);
-dbus_bool_t     bus_service_remove_owner             (BusService                *service,
-						      DBusConnection            *owner,
-                                                      BusTransaction            *transaction,
-                                                      DBusError                 *error);
-dbus_bool_t     bus_service_has_owner                (BusService                *service,
-						      DBusConnection            *owner);
-DBusConnection* bus_service_get_primary_owner        (BusService                *service);
-void            bus_service_set_prohibit_replacement (BusService                *service,
-						      dbus_bool_t                prohibit_replacement);
-dbus_bool_t     bus_service_get_prohibit_replacement (BusService                *service);
-const char*     bus_service_get_name                 (BusService                *service);
 
 #endif /* BUS_SERVICES_H */