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

	* doc/dbus-specification.sgml: require that base service names
	start with ':' and that the base service is created/deleted
	as first and last things a connection does on the bus

	* bus/dispatch.c (check_existent_service_activation): lots more
	work on the activation test; it doesn't fully pass yet...

	* test/test-service.c (main): fix so we don't memleak the
	connection to the message bus
	(filter_func): accept a message asking us to exit

11 files changed, 360 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index 3f867374..8528c672 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2003-04-07  Havoc Pennington  <hp@redhat.com>
+
+	* doc/dbus-specification.sgml: require that base service names 
+	start with ':' and that the base service is created/deleted 
+	as first and last things a connection does on the bus
+
+	* bus/dispatch.c (check_existent_service_activation): lots more 
+	work on the activation test; it doesn't fully pass yet...
+
+	* test/test-service.c (main): fix so we don't memleak the
+	connection to the message bus
+	(filter_func): accept a message asking us to exit
+
 2003-04-06  Havoc Pennington  <hp@pobox.com>
 
 	* qt/Makefile.am (dbusinclude_HEADERS): install dbus-qt.h, 
diff --git a/bus/activation.c b/bus/activation.c
index 425d9c19..8ecf6ecb 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -515,7 +515,7 @@ bus_activation_service_created (BusActivation  *activation,
 	      BUS_SET_OOM (error);
 	      goto error;
 	    }
-
+          
           dbus_message_unref (message);
 	}
 
diff --git a/bus/dispatch.c b/bus/dispatch.c
index ce0d71c6..4761a4bd 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -1034,6 +1034,231 @@ check_nonexistent_service_activation (BusContext     *context,
   return retval;
 }
 
+static dbus_bool_t
+check_service_activated (BusContext     *context,
+                         DBusConnection *connection,
+                         const char     *activated_name,
+                         dbus_bool_t     require_base_service,
+                         DBusMessage    *initial_message,
+                         char          **base_service_p)
+{
+  DBusMessage *message;
+  dbus_bool_t retval;
+  DBusError error;
+  char *base_service;
+  dbus_uint32_t activation_result;
+  dbus_bool_t already_saw_base_created;
+  
+  base_service = NULL;
+  retval = FALSE;
+  
+  dbus_error_init (&error);
+
+  message = initial_message;
+  dbus_message_ref (message);
+  
+  /* This is kind of a mess since we get the creation of
+   * the base service only if the activated service didn't
+   * already exist. Right now the test kills and restarts
+   * the service each time, so the mess is pointless.
+   */
+  already_saw_base_created = FALSE;
+
+ recheck_service_created:
+  if (dbus_message_name_is (message, DBUS_MESSAGE_SERVICE_CREATED))
+    {
+      char *service_name;
+      CheckServiceCreatedData scd;
+          
+      if (!dbus_message_get_args (message, &error,
+                                  DBUS_TYPE_STRING, &service_name,
+                                  DBUS_TYPE_INVALID))
+        {
+          _dbus_warn ("Message %s doesn't have a service name: %s\n",
+                      dbus_message_get_name (message),
+                      error.message);
+          dbus_error_free (&error);
+          goto out;
+        }
+
+      if (!already_saw_base_created && *service_name == ':')
+        {
+          /* This is a base service name, mop up all the
+           * other messages about it
+           */
+              
+          base_service = service_name;
+          service_name = NULL;
+              
+          scd.skip_connection = connection;
+          scd.failed = FALSE;
+          scd.expected_service_name = base_service;
+          bus_test_clients_foreach (check_service_created_foreach,
+                                    &scd);
+              
+          if (scd.failed)
+            goto out;
+
+          already_saw_base_created = TRUE;
+
+          dbus_message_unref (message);
+          message = dbus_connection_pop_message (connection);
+          if (message == NULL)
+            {
+              _dbus_warn ("Expected a ServiceCreated for the activated service, got nothing\n");
+              goto out;
+            }
+              
+          goto recheck_service_created;
+        }
+      else if (require_base_service)
+        {
+          _dbus_warn ("Did not get a ServiceCreated for a base service\n");
+          goto out;
+        }
+
+      if (strcmp (service_name, activated_name) != 0)
+        {
+          _dbus_warn ("Expected to see service %s created, saw %s instead\n",
+                      activated_name, service_name);
+          dbus_free (service_name);
+          goto out;
+        }
+      
+      scd.skip_connection = connection;
+      scd.failed = FALSE;
+      scd.expected_service_name = service_name;
+      bus_test_clients_foreach (check_service_created_foreach,
+                                &scd);
+          
+      dbus_free (service_name);
+
+      if (scd.failed)
+        goto out;
+          
+      dbus_message_unref (message);
+      message = dbus_connection_pop_message (connection);
+      if (message == NULL)
+        {
+          _dbus_warn ("Expected a reply to %s, got nothing\n",
+                      DBUS_MESSAGE_ACTIVATE_SERVICE);
+          goto out;
+        }
+    }
+      
+  if (!dbus_message_name_is (message, DBUS_MESSAGE_ACTIVATE_SERVICE))
+    {
+      _dbus_warn ("Expected reply to %s, got message %s instead\n",
+                  DBUS_MESSAGE_ACTIVATE_SERVICE,
+                  dbus_message_get_name (message));
+      goto out;
+    }
+
+  activation_result = 0;
+  if (!dbus_message_get_args (message, &error,
+                              DBUS_TYPE_UINT32, &activation_result,
+                              DBUS_TYPE_INVALID))
+    {
+      if (!dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
+        {
+          _dbus_warn ("Did not have activation result first argument to %s: %s\n",
+                      DBUS_MESSAGE_ACTIVATE_SERVICE, error.message);
+          dbus_error_free (&error);
+          goto out;
+        }
+
+      dbus_error_free (&error);
+    }
+  else
+    {
+      if (activation_result == DBUS_ACTIVATION_REPLY_ACTIVATED)
+        ; /* Good */
+      else if (activation_result == DBUS_ACTIVATION_REPLY_ALREADY_ACTIVE)
+        ; /* Good also */
+      else
+        {
+          _dbus_warn ("Activation result was 0x%x, no good.\n",
+                      activation_result);
+          goto out;
+        }
+    }
+
+  dbus_message_unref (message);
+  message = NULL;
+      
+  if (!check_no_leftovers (context))
+    {
+      _dbus_warn ("Messages were left over after verifying existent activation results\n");
+      goto out;
+    }
+
+  retval = TRUE;
+
+  if (base_service_p)
+    {
+      *base_service_p = base_service;
+      base_service = NULL;
+    }
+  
+ out:
+  if (message)
+    dbus_message_unref (message);
+
+  if (base_service)
+    dbus_free (base_service);
+  
+  return retval;
+}
+
+static dbus_bool_t
+check_service_deactivated (BusContext     *context,
+                           DBusConnection *connection,
+                           const char     *activated_name,
+                           const char     *base_service)
+{
+  DBusMessage *message;
+  dbus_bool_t retval;
+  DBusError error;
+  CheckServiceDeletedData csdd;
+
+  message = NULL;
+  retval = FALSE;
+  
+  dbus_error_init (&error);
+
+  /* Now we are expecting ServiceDeleted messages for the base
+   * service and the activated_name.  The base service
+   * notification is required to come second.
+   */
+  csdd.expected_service_name = activated_name;
+  csdd.failed = FALSE;
+  bus_test_clients_foreach (check_service_deleted_foreach,
+                            &csdd);      
+
+  if (csdd.failed)
+    goto out;
+      
+  csdd.expected_service_name = base_service;
+  csdd.failed = FALSE;
+  bus_test_clients_foreach (check_service_deleted_foreach,
+                            &csdd);
+
+  if (csdd.failed)
+    goto out;
+      
+  if (!check_no_leftovers (context))
+    {
+      _dbus_warn ("Messages were left over after verifying results of service exiting\n");
+      goto out;
+    }
+  
+ out:
+  if (message)
+    dbus_message_unref (message);
+  
+  return retval;
+}
+
 #define EXISTENT_SERVICE_NAME "org.freedesktop.DBus.TestSuiteEchoService"
 
 /* returns TRUE if the correct thing happens,
@@ -1047,6 +1272,9 @@ check_existent_service_activation (BusContext     *context,
   dbus_int32_t serial;
   dbus_bool_t retval;
   DBusError error;
+  char *base_service;
+
+  base_service = NULL;
   
   dbus_error_init (&error);
   
@@ -1079,7 +1307,7 @@ check_existent_service_activation (BusContext     *context,
   if (dbus_connection_get_dispatch_status (connection) ==
       DBUS_DISPATCH_COMPLETE)
     /* now wait for the message bus to hear back from the activated service */
-    bus_test_run_bus_loop (context);
+    bus_test_run_bus_loop (context, TRUE);
   
   /* and process everything again */
   bus_test_run_everything (context);
@@ -1132,9 +1360,51 @@ check_existent_service_activation (BusContext     *context,
     }
   else
     {
-      _dbus_warn ("Did not expect to successfully activate %s\n",
-                  EXISTENT_SERVICE_NAME);
-      goto out;
+      if (!check_service_activated (context, connection,
+                                    EXISTENT_SERVICE_NAME, TRUE,
+                                    message, &base_service))
+        goto out;
+
+      dbus_message_unref (message);
+      message = NULL;
+  
+      /* Now kill off the test service by sending it a quit message */
+      message = dbus_message_new (EXISTENT_SERVICE_NAME,
+                                  "org.freedesktop.DBus.TestSuiteExit");
+      
+      if (message == NULL)
+        {
+          dbus_free (base_service);
+          return TRUE;
+        }
+      
+      if (!dbus_connection_send (connection, message, &serial))
+        {
+          dbus_message_unref (message);
+          dbus_free (base_service);
+          return TRUE;
+        }
+
+      dbus_message_unref (message);
+      message = NULL;
+
+      /* send message */
+      bus_test_run_clients_loop (TRUE);
+
+      /* read it in and write it out to test service */
+      bus_test_run_bus_loop (context, FALSE);
+      
+      if (dbus_connection_get_dispatch_status (connection) ==
+          DBUS_DISPATCH_COMPLETE)
+        /* now wait for the message bus to hear back from the activated service exiting */
+        bus_test_run_bus_loop (context, TRUE);
+      
+      /* and process everything again */
+      bus_test_run_everything (context);
+
+      if (!check_service_deactivated (context, connection,
+                                      EXISTENT_SERVICE_NAME, base_service))
+        goto out;
     }
 
   retval = TRUE;
@@ -1142,6 +1412,9 @@ check_existent_service_activation (BusContext     *context,
  out:
   if (message)
     dbus_message_unref (message);
+
+  if (base_service)
+    dbus_free (base_service);
   
   return retval;
 }
@@ -1162,7 +1435,7 @@ check_oom_check1_func (void *data)
   
   if (!check_no_leftovers (d->context))
     {
-      _dbus_warn ("Messages were left over, should be covered by test suite");
+      _dbus_warn ("Messages were left over, should be covered by test suite\n");
       return FALSE;
     }
 
diff --git a/bus/test.c b/bus/test.c
index 31c279f9..ded23ba8 100644
--- a/bus/test.c
+++ b/bus/test.c
@@ -297,13 +297,13 @@ bus_test_client_listed (DBusConnection *connection)
 }
 
 void
-bus_test_run_clients_loop (void)
+bus_test_run_clients_loop (dbus_bool_t block_once)
 {
   if (client_loop == NULL)
     return;
   
   /* Do one blocking wait, since we're expecting data */
-  _dbus_loop_iterate (client_loop, TRUE);
+  _dbus_loop_iterate (client_loop, block_once);
 
   /* Then mop everything up */
   while (_dbus_loop_iterate (client_loop, FALSE))
@@ -311,10 +311,11 @@ bus_test_run_clients_loop (void)
 }
 
 void
-bus_test_run_bus_loop (BusContext *context)
+bus_test_run_bus_loop (BusContext *context,
+                       dbus_bool_t block_once)
 {
   /* Do one blocking wait, since we're expecting data */
-  _dbus_loop_iterate (bus_context_get_loop (context), TRUE);
+  _dbus_loop_iterate (bus_context_get_loop (context), block_once);
 
   /* Then mop everything up */
   while (_dbus_loop_iterate (bus_context_get_loop (context), FALSE))
diff --git a/bus/test.h b/bus/test.h
index 5606bb99..3d2a6fda 100644
--- a/bus/test.h
+++ b/bus/test.h
@@ -39,8 +39,9 @@ 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);
-void        bus_test_run_bus_loop     (BusContext                   *context);
-void        bus_test_run_clients_loop (void);
+void        bus_test_run_bus_loop     (BusContext                   *context,
+                                       dbus_bool_t                   block);
+void        bus_test_run_clients_loop (dbus_bool_t                   block);
 void        bus_test_run_everything   (BusContext                   *context);
 BusContext* bus_context_new_test      (const DBusString             *test_data_dir,
                                        const char                   *filename);
diff --git a/dbus/dbus-bus.c b/dbus/dbus-bus.c
index c16ceeb4..74703866 100644
--- a/dbus/dbus-bus.c
+++ b/dbus/dbus-bus.c
@@ -242,7 +242,7 @@ static void
 bus_data_free (void *data)
 {
   BusData *bd = data;
-
+  
   if (bd->is_well_known)
     {
       int i;
@@ -253,7 +253,7 @@ bus_data_free (void *data)
         {
           if (bus_connections[i] == bd->connection)
             bus_connections[i] = NULL;
-
+          
           ++i;
         }
       _DBUS_UNLOCK (bus);
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index e28e45b1..d92268ba 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -977,7 +977,7 @@ dbus_connection_unref (DBusConnection *connection)
   last_unref = (connection->refcount == 0);
 
 #if 0
-  _dbus_verbose ("unref() connection %p count = %d\n", connection, connection->refcount);
+  printf ("unref() connection %p count = %d\n", connection, connection->refcount);
 #endif
   
   dbus_mutex_unlock (connection->mutex);
diff --git a/doc/dbus-specification.sgml b/doc/dbus-specification.sgml
index 3c41068e..3bd49ac4 100644
--- a/doc/dbus-specification.sgml
+++ b/doc/dbus-specification.sgml
@@ -1137,11 +1137,18 @@
         applications.
       </para>
       <para>
-        [FIXME I think we should define the format of the base service name, 
-        and specify that a regular service name can never be in that 
-        format; this allows us to categorically prevent "spoofing" - for 
-        example perhaps the base service name starts with a certain 
-        character that no real service name can start with]
+        Ownership of the base service is a prerequisite for interaction with 
+        the message bus. It logically follows that the base service is always 
+        the first service that an application comes to own, and the last 
+        service that it loses ownership of.
+      </para>
+      <para>
+        Base service names must begin with the character ':' (ASCII colon
+        character); service names that are not base service names must not begin
+        with this character. (The bus must reject any attempt by an application
+        to manually create a service name beginning with ':'.) This restriction
+        categorically prevents "spoofing"; messages sent to a base service name
+        will always go to a single application instance and that instance only.
       </para>
       <para>
         An application can request additional service names to be associated
diff --git a/test/test-service.c b/test/test-service.c
index a1f2ae34..a9a960a1 100644
--- a/test/test-service.c
+++ b/test/test-service.c
@@ -2,6 +2,17 @@
 #include "test-utils.h"
 
 static DBusLoop *loop;
+static dbus_bool_t already_quit;
+
+static void
+quit (void)
+{
+  if (!already_quit)
+    {
+      _dbus_loop_quit (loop);
+      already_quit = TRUE;
+    }
+}
 
 static void
 die (const char *message)
@@ -62,12 +73,14 @@ filter_func (DBusMessageHandler *handler,
              DBusConnection     *connection,
              DBusMessage        *message,
              void               *user_data)
-{
+{  
   if (dbus_message_name_is (message, "org.freedesktop.DBus.TestSuiteEcho"))
     return handle_echo (connection, message);
-  else if (dbus_message_name_is (message, DBUS_MESSAGE_LOCAL_DISCONNECT))
+  else if (dbus_message_name_is (message, "org.freedesktop.DBus.TestSuiteExit") ||
+           dbus_message_name_is (message, DBUS_MESSAGE_LOCAL_DISCONNECT))
     {
-      _dbus_loop_quit (loop);
+      dbus_connection_disconnect (connection);
+      quit ();
       return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS;
     }
   else
@@ -85,6 +98,7 @@ main (int    argc,
   DBusMessageHandler *handler;
   const char *to_handle[] = {
     "org.freedesktop.DBus.TestSuiteEcho",
+    "org.freedesktop.DBus.TestSuiteExit",
     DBUS_MESSAGE_LOCAL_DISCONNECT,
   };
   int result;
@@ -121,11 +135,13 @@ main (int    argc,
       fprintf (stderr, "Failed to acquire service: %s\n",
                error.message);
       dbus_error_free (&error);
-      return 1;
+      exit (1);
     }
   
   _dbus_loop_run (loop);
 
+  test_connection_shutdown (loop, connection);
+  
   dbus_connection_unref (connection);
   
   dbus_message_handler_unref (handler);
@@ -134,6 +150,8 @@ main (int    argc,
   loop = NULL;
   
   dbus_shutdown ();
+
+  printf ("*** Test service exiting\n");
   
   return 0;
 }
diff --git a/test/test-utils.c b/test/test-utils.c
index d7ccd931..62963f38 100644
--- a/test/test-utils.c
+++ b/test/test-utils.c
@@ -176,3 +176,23 @@ test_connection_setup (DBusLoop       *loop,
     cdata_free (cd);
   return FALSE;
 }
+
+void
+test_connection_shutdown (DBusLoop       *loop,
+                          DBusConnection *connection)
+{
+  if (!dbus_connection_set_watch_functions (connection,
+                                            NULL,
+                                            NULL,
+                                            NULL,
+                                            NULL, NULL))
+    _dbus_assert_not_reached ("setting watch functions to NULL failed");
+  
+  if (!dbus_connection_set_timeout_functions (connection,
+                                              NULL,
+                                              NULL,
+                                              NULL,
+                                              NULL, NULL))
+    _dbus_assert_not_reached ("setting timeout functions to NULL failed");
+
+}
diff --git a/test/test-utils.h b/test/test-utils.h
index 2f115c7f..f00a7181 100644
--- a/test/test-utils.h
+++ b/test/test-utils.h
@@ -11,7 +11,10 @@
 
 dbus_bool_t test_connection_setup                 (DBusLoop       *loop,
                                                    DBusConnection *connection);
+void        test_connection_shutdown              (DBusLoop       *loop,
+                                                   DBusConnection *connection);
 void        test_connection_dispatch_all_messages (DBusConnection *connection);
 dbus_bool_t test_connection_dispatch_one_message  (DBusConnection *connection);
 
+
 #endif