2004-11-25 Havoc Pennington <hp@redhat.com>

        The primary change here is to always write() once before adding
	the write watch, which gives us about a 10% performance increase.

	* dbus/dbus-transport-unix.c: a number of modifications to cope
	with removing messages_pending
	(check_write_watch): properly handle
	DBUS_AUTH_STATE_WAITING_FOR_MEMORY; adapt to removal of
	messages_pending stuff
	(check_read_watch): properly handle WAITING_FOR_MEMORY and
	AUTHENTICATED cases
	(unix_handle_watch): after writing, see if the write watch can be
	removed
	(unix_do_iteration): assert that write_watch/read_watch are
	non-NULL rather than testing that they aren't, since they
	aren't allowed to be NULL. check_write_watch() at the end so
	we add the watch if we did not finish writing (e.g. got EAGAIN)

	* dbus/dbus-transport-protected.h: remove messages_pending call,
	since it resulted in too much inefficient watch adding/removing;
	instead we now require that the transport user does an iteration
	after queueing outgoing messages, and after trying the first
	write() we add a write watch if we got EAGAIN or exceeded our
	max bytes to write per iteration setting

	* dbus/dbus-string.c (_dbus_string_validate_signature): add this
	function

	* dbus/dbus-server-unix.c (unix_finalize): the socket name was
	freed and then accessed, valgrind flagged this bug, fix it

	* dbus/dbus-message.c: fix several bugs where HEADER_FIELD_LAST was taken
	as the last valid field plus 1, where really it is equal to the
	last valid field. Corrects some message corruption issues.

	* dbus/dbus-mainloop.c: verbosity changes

	* dbus/dbus-keyring.c (_dbus_keyring_new_homedir): handle OOM
	instead of aborting in one of the test codepaths

	* dbus/dbus-internals.c (_dbus_verbose_real): fix a bug that
	caused not printing the pid ever again if a verbose was missing
	the newline at the end
	(_dbus_header_field_to_string): add HEADER_FIELD_SIGNATURE

	* dbus/dbus-connection.c: verbosity changes;
	(dbus_connection_has_messages_to_send): new function
	(_dbus_connection_message_sent): no longer call transport->messages_pending
	(_dbus_connection_send_preallocated_unlocked): do one iteration to
	try to write() immediately, so we can avoid the write watch. This
	is the core purpose of this patchset
	(_dbus_connection_get_dispatch_status_unlocked): if disconnected,
	dump the outgoing message queue, so nobody will get confused
	trying to send them or thinking stuff is pending to be sent

	* bus/test.c: verbosity changes

	* bus/driver.c: verbosity/assertion changes

	* bus/dispatch.c: a bunch of little tweaks to get it working again
	because this patchset changes when/where you need to block.

3 files changed, 118 insertions, 40 deletions

diff --git a/bus/dispatch.c b/bus/dispatch.c
index 3cd1c3e4..e04d2895 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -2,7 +2,7 @@
 /* dispatch.c  Message dispatcher
  *
  * Copyright (C) 2003  CodeFactory AB
- * Copyright (C) 2003  Red Hat, Inc.
+ * Copyright (C) 2003, 2004  Red Hat, Inc.
  * Copyright (C) 2004  Imendio HB
  *
  * Licensed under the Academic Free License version 2.1
@@ -401,6 +401,12 @@ bus_dispatch_remove_connection (DBusConnection *connection)
 
 #include <stdio.h>
 
+/* This is used to know whether we need to block in order to finish
+ * sending a message, or whether the initial dbus_connection_send()
+ * already flushed the queue.
+ */
+#define SEND_PENDING(connection) (dbus_connection_has_messages_to_send (connection))
+
 typedef dbus_bool_t (* Check1Func) (BusContext     *context);
 typedef dbus_bool_t (* Check2Func) (BusContext     *context,
                                     DBusConnection *connection);
@@ -409,8 +415,11 @@ static dbus_bool_t check_no_leftovers (BusContext *context);
 
 static void
 block_connection_until_message_from_bus (BusContext     *context,
-                                         DBusConnection *connection)
+                                         DBusConnection *connection,
+                                         const char     *what_is_expected)
 {
+  _dbus_verbose ("expecting: %s\n", what_is_expected);
+  
   while (dbus_connection_get_dispatch_status (connection) ==
          DBUS_DISPATCH_COMPLETE &&
          dbus_connection_get_is_connected (connection))
@@ -420,6 +429,20 @@ block_connection_until_message_from_bus (BusContext     *context,
     }
 }
 
+static void
+spin_connection_until_authenticated (BusContext     *context,
+                                     DBusConnection *connection)
+{
+  _dbus_verbose ("Spinning to auth connection %p\n", connection);
+  while (!dbus_connection_get_is_authenticated (connection) &&
+         dbus_connection_get_is_connected (connection))
+    {
+      bus_test_run_bus_loop (context, FALSE);
+      bus_test_run_clients_loop (FALSE);
+    }
+  _dbus_verbose (" ... done spinning to auth connection %p\n", connection);
+}
+
 /* compensate for fact that pop_message() can return #NULL due to OOM */
 static DBusMessage*
 pop_message_waiting_for_memory (DBusConnection *connection)
@@ -737,24 +760,46 @@ check_hello_message (BusContext     *context,
   if (message == NULL)
     return TRUE;
 
+  dbus_connection_ref (connection); /* because we may get disconnected */
+  
   if (!dbus_connection_send (connection, message, &serial))
     {
       dbus_message_unref (message);
+      dbus_connection_unref (connection);
       return TRUE;
     }
 
+  _dbus_assert (dbus_message_has_signature (message, ""));
+  
   dbus_message_unref (message);
   message = NULL;
 
+  if (!dbus_connection_get_is_connected (connection))
+    {
+      _dbus_verbose ("connection was disconnected (presumably auth failed)\n");
+      
+      dbus_connection_unref (connection);
+      
+      return TRUE;
+    }
+  
   /* send our message */
-  bus_test_run_clients_loop (TRUE);
+  bus_test_run_clients_loop (SEND_PENDING (connection));
 
-  dbus_connection_ref (connection); /* because we may get disconnected */
-  block_connection_until_message_from_bus (context, connection);
+  if (!dbus_connection_get_is_connected (connection))
+    {
+      _dbus_verbose ("connection was disconnected (presumably auth failed)\n");
+      
+      dbus_connection_unref (connection);
+      
+      return TRUE;
+    }
+  
+  block_connection_until_message_from_bus (context, connection, "reply to Hello");
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected (presumably auth failed)\n");
       
       dbus_connection_unref (connection);
       
@@ -945,14 +990,14 @@ check_double_hello_message (BusContext     *context,
   message = NULL;
 
   /* send our message */
-  bus_test_run_clients_loop (TRUE);
+  bus_test_run_clients_loop (SEND_PENDING (connection));
 
   dbus_connection_ref (connection); /* because we may get disconnected */
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply to Hello");
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       
       dbus_connection_unref (connection);
       
@@ -1044,17 +1089,17 @@ check_get_connection_unix_user (BusContext     *context,
     }
 
   /* send our message */
-  bus_test_run_clients_loop (TRUE);
+  bus_test_run_clients_loop (SEND_PENDING (connection));
 
   dbus_message_unref (message);
   message = NULL;
 
   dbus_connection_ref (connection); /* because we may get disconnected */
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply to GetConnectionUnixUser");
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       
       dbus_connection_unref (connection);
       
@@ -1181,17 +1226,17 @@ check_get_connection_unix_process_id (BusContext     *context,
     }
 
   /* send our message */
-  bus_test_run_clients_loop (TRUE);
+  bus_test_run_clients_loop (SEND_PENDING (connection));
 
   dbus_message_unref (message);
   message = NULL;
 
   dbus_connection_ref (connection); /* because we may get disconnected */
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply to GetConnectionUnixProcessID");
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       
       dbus_connection_unref (connection);
       
@@ -1331,15 +1376,25 @@ check_add_match_all (BusContext     *context,
   dbus_message_unref (message);
   message = NULL;
 
+  dbus_connection_ref (connection); /* because we may get disconnected */
+  
   /* send our message */
-  bus_test_run_clients_loop (TRUE);
+  bus_test_run_clients_loop (SEND_PENDING (connection));
 
-  dbus_connection_ref (connection); /* because we may get disconnected */
-  block_connection_until_message_from_bus (context, connection);
+  if (!dbus_connection_get_is_connected (connection))
+    {
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
+      
+      dbus_connection_unref (connection);
+      
+      return TRUE;
+    }
+  
+  block_connection_until_message_from_bus (context, connection, "reply to AddMatch");
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       
       dbus_connection_unref (connection);
       
@@ -1435,6 +1490,8 @@ check_hello_connection (BusContext *context)
       return TRUE;
     }
 
+  spin_connection_until_authenticated (context, connection);
+  
   if (!check_hello_message (context, connection))
     return FALSE;
   
@@ -1496,12 +1553,12 @@ check_nonexistent_service_activation (BusContext     *context,
   message = NULL;
 
   bus_test_run_everything (context);
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply to ActivateService on nonexistent");
   bus_test_run_everything (context);
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       return TRUE;
     }
   
@@ -1590,12 +1647,12 @@ check_nonexistent_service_auto_activation (BusContext     *context,
   message = NULL;
 
   bus_test_run_everything (context);
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply to Echo");
   bus_test_run_everything (context);
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       return TRUE;
     }
   
@@ -2116,7 +2173,7 @@ check_send_exit_to_service (BusContext     *context,
   message = NULL;
 
   /* send message */
-  bus_test_run_clients_loop (TRUE);
+  bus_test_run_clients_loop (SEND_PENDING (connection));
 
   /* read it in and write it out to test service */
   bus_test_run_bus_loop (context, FALSE);
@@ -2134,7 +2191,7 @@ check_send_exit_to_service (BusContext     *context,
   if (!got_error)
     {
       /* If no error, wait for the test service to exit */
-      block_connection_until_message_from_bus (context, connection);
+      block_connection_until_message_from_bus (context, connection, "test service to exit");
               
       bus_test_run_everything (context);
     }
@@ -2394,13 +2451,13 @@ check_existent_service_activation (BusContext     *context,
   /* now wait for the message bus to hear back from the activated
    * service.
    */
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "activated service to connect");
 
   bus_test_run_everything (context);
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       return TRUE;
     }
   
@@ -2458,7 +2515,7 @@ check_existent_service_activation (BusContext     *context,
       message = NULL;
 
       /* We may need to block here for the test service to exit or finish up */
-      block_connection_until_message_from_bus (context, connection);
+      block_connection_until_message_from_bus (context, connection, "test service to exit or finish up");
       
       message = dbus_connection_borrow_message (connection);
       if (message == NULL)
@@ -2514,7 +2571,7 @@ check_existent_service_activation (BusContext     *context,
 	     */
 	    if (message_kind != GOT_ERROR)
 	      {
-		block_connection_until_message_from_bus (context, connection);
+		block_connection_until_message_from_bus (context, connection, "error about service exiting");
               
 		/* and process everything again */
 		bus_test_run_everything (context);
@@ -2608,12 +2665,12 @@ check_segfault_service_activation (BusContext     *context,
   message = NULL;
 
   bus_test_run_everything (context);
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply to activating segfault service");
   bus_test_run_everything (context);
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       return TRUE;
     }
   
@@ -2703,12 +2760,12 @@ check_segfault_service_auto_activation (BusContext     *context,
   message = NULL;
 
   bus_test_run_everything (context);
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply to Echo on segfault service");
   bus_test_run_everything (context);
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       return TRUE;
     }
   
@@ -2814,12 +2871,12 @@ check_existent_service_auto_activation (BusContext     *context,
   /* now wait for the message bus to hear back from the activated
    * service.
    */
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply to Echo on existent service");
   bus_test_run_everything (context);
 
   if (!dbus_connection_get_is_connected (connection))
     {
-      _dbus_verbose ("connection was disconnected\n");
+      _dbus_verbose ("connection was disconnected: %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
       return TRUE;
     }
 
@@ -2849,7 +2906,7 @@ check_existent_service_auto_activation (BusContext     *context,
       message = NULL;
 
       /* We may need to block here for the test service to exit or finish up */
-      block_connection_until_message_from_bus (context, connection);
+      block_connection_until_message_from_bus (context, connection, "service to exit");
 
       /* Should get a service creation notification for the activated
        * service name, or a service deletion on the base service name
@@ -2924,7 +2981,7 @@ check_existent_service_auto_activation (BusContext     *context,
   /* Note: if this test is run in OOM mode, it will block when the bus
    * doesn't send a reply due to OOM.
    */
-  block_connection_until_message_from_bus (context, connection);
+  block_connection_until_message_from_bus (context, connection, "reply from echo message after auto-activation");
       
   message = pop_message_waiting_for_memory (connection);
   if (message == NULL)
@@ -3064,6 +3121,8 @@ bus_dispatch_test (const DBusString *test_data_dir)
   if (!bus_setup_debug_client (foo))
     _dbus_assert_not_reached ("could not set up connection");
 
+  spin_connection_until_authenticated (context, foo);
+  
   if (!check_hello_message (context, foo))
     _dbus_assert_not_reached ("hello message failed");
 
@@ -3080,6 +3139,8 @@ bus_dispatch_test (const DBusString *test_data_dir)
   if (!bus_setup_debug_client (bar))
     _dbus_assert_not_reached ("could not set up connection");
 
+  spin_connection_until_authenticated (context, bar);
+  
   if (!check_hello_message (context, bar))
     _dbus_assert_not_reached ("hello message failed");
 
@@ -3093,6 +3154,8 @@ bus_dispatch_test (const DBusString *test_data_dir)
   if (!bus_setup_debug_client (baz))
     _dbus_assert_not_reached ("could not set up connection");
 
+  spin_connection_until_authenticated (context, baz);
+  
   if (!check_hello_message (context, baz))
     _dbus_assert_not_reached ("hello message failed");
 
@@ -3176,6 +3239,8 @@ bus_dispatch_sha1_test (const DBusString *test_data_dir)
   if (!bus_setup_debug_client (foo))
     _dbus_assert_not_reached ("could not set up connection");
 
+  spin_connection_until_authenticated (context, foo);
+  
   if (!check_hello_message (context, foo))
     _dbus_assert_not_reached ("hello message failed");
 
diff --git a/bus/driver.c b/bus/driver.c
index b426912f..dc640396 100644
--- a/bus/driver.c
+++ b/bus/driver.c
@@ -2,7 +2,7 @@
 /* driver.c  Bus client (driver)
  *
  * Copyright (C) 2003 CodeFactory AB
- * Copyright (C) 2003 Red Hat, Inc.
+ * Copyright (C) 2003, 2004 Red Hat, Inc.
  *
  * Licensed under the Academic Free License version 2.1
  * 
@@ -51,7 +51,8 @@ bus_driver_send_service_owner_changed (const char     *service_name,
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
   
-  _dbus_verbose ("sending service owner changed: %s [%s -> %s]", service_name, 
+  _dbus_verbose ("sending service owner changed: %s [%s -> %s]\n",
+                 service_name, 
                  old_owner ? old_owner : null_service, 
                  new_owner ? new_owner : null_service);
 
@@ -75,6 +76,8 @@ bus_driver_send_service_owner_changed (const char     *service_name,
                                  DBUS_TYPE_INVALID))
     goto oom;
 
+  _dbus_assert (dbus_message_has_signature (message, "sss"));
+  
   retval = bus_dispatch_matches (transaction, NULL, NULL, message, error);
   dbus_message_unref (message);
 
@@ -346,6 +349,8 @@ bus_driver_send_welcome_message (DBusConnection *connection,
       return FALSE;
     }
 
+  _dbus_assert (dbus_message_has_signature (welcome, "s"));
+  
   if (!bus_transaction_send_from_driver (transaction, connection, welcome))
     {
       dbus_message_unref (welcome);
diff --git a/bus/test.c b/bus/test.c
index 4071fb18..90d044ec 100644
--- a/bus/test.c
+++ b/bus/test.c
@@ -231,10 +231,12 @@ bus_test_client_listed (DBusConnection *connection)
 
 void
 bus_test_run_clients_loop (dbus_bool_t block_once)
-{
+{  
   if (client_loop == NULL)
     return;
 
+  _dbus_verbose ("---> Dispatching on \"client side\"\n");
+  
   /* dispatch before we block so pending dispatches
    * won't make our block return early
    */
@@ -250,12 +252,16 @@ bus_test_run_clients_loop (dbus_bool_t block_once)
   /* Then mop everything up */
   while (_dbus_loop_iterate (client_loop, FALSE))
     ;
+
+  _dbus_verbose ("---> Done dispatching on \"client side\"\n");
 }
 
 void
 bus_test_run_bus_loop (BusContext *context,
                        dbus_bool_t block_once)
 {
+  _dbus_verbose ("---> Dispatching on \"server side\"\n");
+  
   /* dispatch before we block so pending dispatches
    * won't make our block return early
    */
@@ -271,6 +277,8 @@ bus_test_run_bus_loop (BusContext *context,
   /* Then mop everything up */
   while (_dbus_loop_iterate (bus_context_get_loop (context), FALSE))
     ;
+
+  _dbus_verbose ("---> Done dispatching on \"server side\"\n");
 }
 
 void