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

	All tests pass, no memleaks, no valgrind complaints.

	* bus/test.c: refcount handler_slot

	* bus/connection.c (bus_connections_new): refcount
	connection_data_slot

	* dbus/dbus-auth-script.c (_dbus_auth_script_run): delete unused
	bytes so that auth scripts pass.

	* bus/dispatch.c: init message_handler_slot so it gets allocated
	properly

	* bus/dispatch.c (message_handler_slot_ref): fix memleak

	* dbus/dbus-server-debug-pipe.c (_dbus_server_debug_pipe_new):
	dealloc server_pipe_hash when no longer used for benefit of
	leak checking

	* dbus/dbus-auth.c (process_command): memleak fix

	* bus/dispatch.c (check_hello_message): memleak fix

9 files changed, 253 insertions, 63 deletions

diff --git a/ChangeLog b/ChangeLog
index a4784233..0796f2a0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,28 @@
+2003-03-17  Havoc Pennington  <hp@pobox.com>
+
+	All tests pass, no memleaks, no valgrind complaints.
+	
+	* bus/test.c: refcount handler_slot
+
+	* bus/connection.c (bus_connections_new): refcount
+	connection_data_slot
+
+	* dbus/dbus-auth-script.c (_dbus_auth_script_run): delete unused
+	bytes so that auth scripts pass.
+
+	* bus/dispatch.c: init message_handler_slot so it gets allocated
+	properly
+
+	* bus/dispatch.c (message_handler_slot_ref): fix memleak
+
+	* dbus/dbus-server-debug-pipe.c (_dbus_server_debug_pipe_new):
+	dealloc server_pipe_hash when no longer used for benefit of
+	leak checking
+
+	* dbus/dbus-auth.c (process_command): memleak fix
+
+	* bus/dispatch.c (check_hello_message): memleak fix
+
 2003-03-16  Havoc Pennington  <hp@pobox.com>
 
 	* dbus/dbus-bus.c (ensure_bus_data): fix double-unref of the data slot
diff --git a/bus/connection.c b/bus/connection.c
index a8616683..1b59819f 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -37,6 +37,7 @@ struct BusConnections
 };
 
 static int connection_data_slot = -1;
+static int connection_data_slot_refcount = 0;
 
 typedef struct
 {
@@ -51,6 +52,39 @@ typedef struct
 
 #define BUS_CONNECTION_DATA(connection) (dbus_connection_get_data ((connection), connection_data_slot))
 
+static dbus_bool_t
+connection_data_slot_ref (void)
+{
+  if (connection_data_slot < 0)
+    {
+      connection_data_slot = dbus_connection_allocate_data_slot ();
+      
+      if (connection_data_slot < 0)
+        return FALSE;
+
+      _dbus_assert (connection_data_slot_refcount == 0);
+    }  
+
+  connection_data_slot_refcount += 1;
+
+  return TRUE;
+
+}
+
+static void
+connection_data_slot_unref (void)
+{
+  _dbus_assert (connection_data_slot_refcount > 0);
+
+  connection_data_slot_refcount -= 1;
+  
+  if (connection_data_slot_refcount == 0)
+    {
+      dbus_connection_free_data_slot (connection_data_slot);
+      connection_data_slot = -1;
+    }
+}
+
 void
 bus_connection_disconnected (DBusConnection *connection)
 {
@@ -209,6 +243,7 @@ free_connection_data (void *data)
 
   if (d->oom_preallocated)
     dbus_connection_free_preallocated_send (d->connection, d->oom_preallocated);
+
   if (d->oom_message)
     dbus_message_unref (d->oom_message);
   
@@ -222,17 +257,15 @@ bus_connections_new (BusContext *context)
 {
   BusConnections *connections;
 
-  if (connection_data_slot < 0)
-    {
-      connection_data_slot = dbus_connection_allocate_data_slot ();
-      
-      if (connection_data_slot < 0)
-        return NULL;
-    }
+  if (!connection_data_slot_ref ())
+    return NULL;
 
   connections = dbus_new0 (BusConnections, 1);
   if (connections == NULL)
-    return NULL;
+    {
+      connection_data_slot_unref ();
+      return NULL;
+    }
   
   connections->refcount = 1;
   connections->context = context;
@@ -268,7 +301,9 @@ bus_connections_unref (BusConnections *connections)
       
       _dbus_list_clear (&connections->list);
       
-      dbus_free (connections);      
+      dbus_free (connections);
+
+      connection_data_slot_unref ();
     }
 }
 
@@ -286,6 +321,8 @@ bus_connections_setup_connection (BusConnections *connections,
 
   d->connections = connections;
   d->connection = connection;
+
+  _dbus_assert (connection_data_slot >= 0);
   
   if (!dbus_connection_set_data (connection,
                                  connection_data_slot,
@@ -329,6 +366,11 @@ bus_connections_setup_connection (BusConnections *connections,
  out:
   if (!retval)
     {
+      if (!dbus_connection_set_data (connection,
+                                     connection_data_slot,
+                                     NULL, NULL))
+        _dbus_assert_not_reached ("failed to set connection data to null");
+        
       if (!dbus_connection_set_watch_functions (connection,
                                                 NULL, NULL, NULL,
                                                 connection,
diff --git a/bus/dispatch.c b/bus/dispatch.c
index ffb7bd9f..5365a11d 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -33,7 +33,7 @@
 #include <dbus/dbus-internals.h>
 #include <string.h>
 
-static int message_handler_slot;
+static int message_handler_slot = -1;
 static int message_handler_slot_refcount;
 
 typedef struct
@@ -157,7 +157,7 @@ bus_dispatch (DBusConnection *connection,
   
   transaction = NULL;
   dbus_error_init (&error);
-
+  
   context = bus_connection_get_context (connection);
   _dbus_assert (context != NULL);
   
@@ -334,10 +334,15 @@ bus_dispatch_message_handler (DBusMessageHandler *handler,
 static dbus_bool_t
 message_handler_slot_ref (void)
 {
-  message_handler_slot = dbus_connection_allocate_data_slot ();
-
   if (message_handler_slot < 0)
-    return FALSE;
+    {
+      message_handler_slot = dbus_connection_allocate_data_slot ();
+      
+      if (message_handler_slot < 0)
+        return FALSE;
+
+      _dbus_assert (message_handler_slot_refcount == 0);
+    }  
 
   message_handler_slot_refcount += 1;
 
@@ -348,7 +353,9 @@ 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);
@@ -356,6 +363,18 @@ message_handler_slot_unref (void)
     }
 }
 
+static void
+free_message_handler (void *data)
+{
+  DBusMessageHandler *handler = data;
+  
+  _dbus_assert (message_handler_slot >= 0);
+  _dbus_assert (message_handler_slot_refcount > 0);
+  
+  dbus_message_handler_unref (handler);
+  message_handler_slot_unref ();
+}
+
 dbus_bool_t
 bus_dispatch_add_connection (DBusConnection *connection)
 {
@@ -369,7 +388,7 @@ bus_dispatch_add_connection (DBusConnection *connection)
     {
       message_handler_slot_unref ();
       return FALSE;
-    }
+    }    
   
   if (!dbus_connection_add_filter (connection, handler))
     {
@@ -379,12 +398,14 @@ bus_dispatch_add_connection (DBusConnection *connection)
       return FALSE;
     }
 
+  _dbus_assert (message_handler_slot >= 0);
+  _dbus_assert (message_handler_slot_refcount > 0);
+  
   if (!dbus_connection_set_data (connection,
 				 message_handler_slot,
 				 handler,
-				 (DBusFreeFunction)dbus_message_handler_unref))
+                                 free_message_handler))
     {
-      dbus_connection_remove_filter (connection, handler);
       dbus_message_handler_unref (handler);
       message_handler_slot_unref ();
 
@@ -403,8 +424,6 @@ bus_dispatch_remove_connection (DBusConnection *connection)
   dbus_connection_set_data (connection,
 			    message_handler_slot,
 			    NULL, NULL);
-
-  message_handler_slot_unref ();
 }
 
 #ifdef DBUS_BUILD_TESTS
@@ -551,6 +570,23 @@ kill_client_connection (BusContext     *context,
     _dbus_assert_not_reached ("stuff left in message queues after disconnecting a client");
 }
 
+static void
+kill_client_connection_unchecked (DBusConnection *connection)
+{
+  /* This kills the connection without expecting it to affect
+   * the rest of the bus.
+   */  
+  _dbus_verbose ("Unchecked kill of connection %p\n", connection);
+
+  dbus_connection_ref (connection);
+  dbus_connection_disconnect (connection);
+  /* dispatching disconnect handler will unref once */
+  if (bus_connection_dispatch_one_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));
+}
+
 typedef struct
 {
   dbus_bool_t failed;
@@ -691,7 +727,10 @@ check_hello_message (BusContext     *context,
     return TRUE;
 
   if (!dbus_connection_send (connection, message, &serial))
-    return TRUE;
+    {
+      dbus_message_unref (message);
+      return TRUE;
+    }
 
   dbus_message_unref (message);
   message = NULL;
@@ -870,15 +909,7 @@ check_hello_connection (BusContext *context)
       /* 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 (bus_connection_dispatch_one_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;
+      kill_client_connection_unchecked (connection);
     }
   else
     {
@@ -928,6 +959,9 @@ check1_try_iterations (BusContext *context,
     }
 
   _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
+
+  _dbus_verbose ("=================\n%s: all iterations passed\n=================\n",
+                 description);
 }
 
 dbus_bool_t
@@ -981,20 +1015,11 @@ bus_dispatch_test (const DBusString *test_data_dir)
   check1_try_iterations (context, "create_and_hello",
                          check_hello_connection);
   
-  dbus_connection_disconnect (foo);
-  if (bus_connection_dispatch_one_message (foo))
-    _dbus_assert_not_reached ("extra message in queue");
-  _dbus_assert (!bus_test_client_listed (foo));
-
-  dbus_connection_disconnect (bar);
-  if (bus_connection_dispatch_one_message (bar))
-    _dbus_assert_not_reached ("extra message in queue");
-  _dbus_assert (!bus_test_client_listed (bar));
+  _dbus_verbose ("Disconnecting foo, bar, and baz\n");
 
-  dbus_connection_disconnect (baz);
-  if (bus_connection_dispatch_one_message (baz))
-    _dbus_assert_not_reached ("extra message in queue");
-  _dbus_assert (!bus_test_client_listed (baz));
+  kill_client_connection_unchecked (foo);
+  kill_client_connection_unchecked (bar);
+  kill_client_connection_unchecked (baz);
 
   bus_context_unref (context);
   
diff --git a/bus/test.c b/bus/test.c
index 7de1d811..bc195ef1 100644
--- a/bus/test.c
+++ b/bus/test.c
@@ -112,6 +112,49 @@ client_disconnect_handler (DBusMessageHandler *handler,
 }
 
 static int handler_slot = -1;
+static int handler_slot_refcount = 0;
+
+static dbus_bool_t
+handler_slot_ref (void)
+{
+  if (handler_slot < 0)
+    {
+      handler_slot = dbus_connection_allocate_data_slot ();
+      
+      if (handler_slot < 0)
+        return FALSE;
+
+      _dbus_assert (handler_slot_refcount == 0);
+    }  
+
+  handler_slot_refcount += 1;
+
+  return TRUE;
+
+}
+
+static void
+handler_slot_unref (void)
+{
+  _dbus_assert (handler_slot_refcount > 0);
+
+  handler_slot_refcount -= 1;
+  
+  if (handler_slot_refcount == 0)
+    {
+      dbus_connection_free_data_slot (handler_slot);
+      handler_slot = -1;
+    }
+}
+
+static void
+free_handler (void *data)
+{
+  DBusMessageHandler *handler = data;
+
+  dbus_message_handler_unref (handler);
+  handler_slot_unref ();
+}
 
 dbus_bool_t
 bus_setup_debug_client (DBusConnection *connection)
@@ -119,11 +162,6 @@ 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);
@@ -160,12 +198,17 @@ bus_setup_debug_client (DBusConnection *connection)
   if (!_dbus_list_append (&clients, connection))
     goto out;
 
-  /* Set up handler to be destroyed */
+  if (!handler_slot_ref ())
+    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;
+                                 free_handler))
+    {
+      handler_slot_unref ();
+      goto out;
+    }
   
   retval = TRUE;
   
diff --git a/dbus/dbus-auth-script.c b/dbus/dbus-auth-script.c
index d954c8dd..3aaf5688 100644
--- a/dbus/dbus-auth-script.c
+++ b/dbus/dbus-auth-script.c
@@ -541,6 +541,7 @@ _dbus_auth_script_run (const DBusString *filename)
           
           if (_dbus_string_equal (&expected, unused))
             {
+              _dbus_auth_delete_unused_bytes (auth);
               _dbus_string_free (&expected);
             }
           else
diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c
index 9e2b1d95..8f8aec44 100644
--- a/dbus/dbus-auth.c
+++ b/dbus/dbus-auth.c
@@ -1665,6 +1665,7 @@ process_command (DBusAuth *auth)
 
   if (!_dbus_string_init (&args, _DBUS_INT_MAX))
     {
+      _dbus_string_free (&command);
       auth->needed_memory = TRUE;
       return FALSE;
     }
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index ed677e74..d70ff716 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -866,6 +866,8 @@ _dbus_connection_last_unref (DBusConnection *connection)
   DBusHashIter iter;
   DBusList *link;
 
+  _dbus_verbose ("Finalizing connection %p\n", connection);
+  
   _dbus_assert (connection->refcount == 0);
   
   /* You have to disconnect the connection before unref:ing it. Otherwise
@@ -969,6 +971,10 @@ dbus_connection_unref (DBusConnection *connection)
   connection->refcount -= 1;
   last_unref = (connection->refcount == 0);
 
+#if 0
+  _dbus_verbose ("unref() connection %p count = %d\n", connection, connection->refcount);
+#endif
+  
   dbus_mutex_unlock (connection->mutex);
 
   if (last_unref)
diff --git a/dbus/dbus-dataslot.c b/dbus/dbus-dataslot.c
index 53fb9e4c..46e2bfc1 100644
--- a/dbus/dbus-dataslot.c
+++ b/dbus/dbus-dataslot.c
@@ -57,6 +57,10 @@ _dbus_data_slot_allocator_init (DBusDataSlotAllocator *allocator)
  * Allocates an integer ID to be used for storing data
  * in a #DBusDataSlotList.
  *
+ * @todo all over the code we have foo_slot and foo_slot_refcount,
+ * would be better to add an interface for that to
+ * DBusDataSlotAllocator so it isn't cut-and-pasted everywhere.
+ * 
  * @param allocator the allocator
  * @returns the integer ID, or -1 on failure
  */
@@ -103,6 +107,9 @@ _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator)
 
   _dbus_assert (slot >= 0);
   _dbus_assert (slot < allocator->n_allocated_slots);
+
+  _dbus_verbose ("Allocated slot %d on allocator %p total %d slots allocated %d used\n",
+                 slot, allocator, allocator->n_allocated_slots, allocator->n_used_slots);
   
  out:
   dbus_mutex_unlock (allocator->lock);
@@ -137,6 +144,9 @@ _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator,
       allocator->allocated_slots = NULL;
       allocator->n_allocated_slots = 0;
     }
+
+  _dbus_verbose ("Freed slot %d on allocator %p total %d allocated %d used\n",
+                 slot, allocator, allocator->n_allocated_slots, allocator->n_used_slots);
   
   dbus_mutex_unlock (allocator->lock);
 }
@@ -243,6 +253,7 @@ _dbus_data_slot_list_get  (DBusDataSlotAllocator *allocator,
    */
   if (!dbus_mutex_lock (allocator->lock))
     return FALSE;
+  _dbus_assert (slot >= 0);
   _dbus_assert (slot < allocator->n_allocated_slots);
   _dbus_assert (allocator->allocated_slots[slot] == slot);
   dbus_mutex_unlock (allocator->lock);
diff --git a/dbus/dbus-server-debug-pipe.c b/dbus/dbus-server-debug-pipe.c
index 46e78ddd..c9a25020 100644
--- a/dbus/dbus-server-debug-pipe.c
+++ b/dbus/dbus-server-debug-pipe.c
@@ -59,13 +59,48 @@ struct DBusServerDebugPipe
   dbus_bool_t disconnected; /**< TRUE if disconnect has been called */
 };
 
+/* FIXME not threadsafe (right now the test suite doesn't use threads anyhow ) */
 static DBusHashTable *server_pipe_hash;
+static int server_pipe_hash_refcount = 0;
 
+static dbus_bool_t
+pipe_hash_ref (void)
+{
+  if (!server_pipe_hash)
+    {
+      _dbus_assert (server_pipe_hash_refcount == 0);
+      
+      server_pipe_hash = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, NULL);
+
+      if (!server_pipe_hash)
+        return FALSE;
+    }
+
+  server_pipe_hash_refcount = 1;
+
+  return TRUE;
+}
+
+static void
+pipe_hash_unref (void)
+{
+  _dbus_assert (server_pipe_hash != NULL);
+  _dbus_assert (server_pipe_hash_refcount > 0);
+
+  server_pipe_hash_refcount -= 1;
+  if (server_pipe_hash_refcount == 0)
+    {
+      _dbus_hash_table_unref (server_pipe_hash);
+      server_pipe_hash = NULL;
+    }
+}
 
 static void
 debug_finalize (DBusServer *server)
 {
   DBusServerDebugPipe *debug_server = (DBusServerDebugPipe*) server;
+
+  pipe_hash_unref ();
   
   _dbus_server_finalize_base (server);
 
@@ -107,27 +142,23 @@ _dbus_server_debug_pipe_new (const char     *server_name,
 {
   DBusServerDebugPipe *debug_server;
 
-  if (!server_pipe_hash)
-    {
-      server_pipe_hash = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, NULL);
-
-      if (!server_pipe_hash)
-	{
-	  dbus_set_result (result, DBUS_RESULT_NO_MEMORY);
-	  return NULL;
-	}
-    }
-
+  if (!pipe_hash_ref ())
+    return NULL;
+  
   if (_dbus_hash_table_lookup_string (server_pipe_hash, server_name) != NULL)
     {
       dbus_set_result (result, DBUS_RESULT_ADDRESS_IN_USE);
+      pipe_hash_unref ();
       return NULL;
     }
   
   debug_server = dbus_new0 (DBusServerDebugPipe, 1);
 
   if (debug_server == NULL)
-    return NULL;
+    {
+      pipe_hash_unref ();
+      return NULL;
+    }
 
   debug_server->name = _dbus_strdup (server_name);
   if (debug_server->name == NULL)
@@ -136,6 +167,9 @@ _dbus_server_debug_pipe_new (const char     *server_name,
       dbus_free (debug_server);
 
       dbus_set_result (result, DBUS_RESULT_NO_MEMORY);
+
+      pipe_hash_unref ();
+      return NULL;
     }
   
   if (!_dbus_server_init_base (&debug_server->base,
@@ -146,6 +180,7 @@ _dbus_server_debug_pipe_new (const char     *server_name,
 
       dbus_set_result (result, DBUS_RESULT_NO_MEMORY);
 
+      pipe_hash_unref ();
       return NULL;
     }
 
@@ -159,6 +194,7 @@ _dbus_server_debug_pipe_new (const char     *server_name,
 
       dbus_set_result (result, DBUS_RESULT_NO_MEMORY);
 
+      pipe_hash_unref ();
       return NULL;
     }