2005-02-13 Havoc Pennington <hp@redhat.com>

	* dbus/dbus-connection.c (_dbus_connection_acquire_dispatch)
	(_dbus_connection_release_dispatch)
	(_dbus_connection_acquire_io_path)
	(_dbus_connection_release_io_path): make the mutex and condvar
	control access to the "acquired" flag. Drop the connection lock
	while waiting on the condvar. Hopefully these are baby steps in
	roughly the right direction.

diff --git a/ChangeLog b/ChangeLog
index 9f3a653..34e0945 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2005-02-13  Havoc Pennington  <hp@redhat.com>
 
+       * dbus/dbus-connection.c (_dbus_connection_acquire_dispatch)
+       (_dbus_connection_release_dispatch)
+       (_dbus_connection_acquire_io_path)
+       (_dbus_connection_release_io_path): make the mutex and condvar
+       control access to the "acquired" flag. Drop the connection lock
+       while waiting on the condvar. Hopefully these are baby steps in
+       roughly the right direction.
+
+2005-02-13  Havoc Pennington  <hp@redhat.com>
+
        * dbus/dbus-connection.c: use separate mutexes for the condition
        variables; this is some kind of baseline for sanity, but the
        condition variables still aren't used correctly afaict

diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 0cfb265..3cf3be7 100644 (file)
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -188,10 +188,10 @@ struct DBusConnection
 
   DBusMutex *mutex; /**< Lock on the entire DBusConnection */
 
-  DBusMutex *dispatch_mutex;     /**< Protects dispatch() */
-  DBusCondVar *dispatch_cond;    /**< Notify when dispatch_mutex is available */
-  DBusMutex *io_path_mutex;      /**< Protects transport io path */
-  DBusCondVar *io_path_cond;     /**< Notify when io_path_mutex is available */
+  DBusMutex *dispatch_mutex;     /**< Protects dispatch_acquired */
+  DBusCondVar *dispatch_cond;    /**< Notify when dispatch_acquired is available */
+  DBusMutex *io_path_mutex;      /**< Protects io_path_acquired */
+  DBusCondVar *io_path_cond;     /**< Notify when io_path_acquired is available */
   
   DBusList *outgoing_messages; /**< Queue of messages we need to send, send the end of the list first. */
   DBusList *incoming_messages; /**< Queue of messages we have received, end of the list received most recently. */
@@ -908,32 +908,63 @@ static dbus_bool_t
 _dbus_connection_acquire_io_path (DBusConnection *connection,
                                  int timeout_milliseconds)
 {
-  dbus_bool_t res = TRUE;
+  dbus_bool_t we_acquired;
+  
+  HAVE_LOCK_CHECK (connection);
+
+  /* We don't want the connection to vanish */
+  _dbus_connection_ref_unlocked (connection);
+
+  /* We will only touch io_path_acquired which is protected by our mutex */
+  CONNECTION_UNLOCK (connection);
+  
+  _dbus_verbose ("%s locking io_path_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_lock (connection->io_path_mutex);
 
   _dbus_verbose ("%s start connection->io_path_acquired = %d timeout = %d\n",
                  _DBUS_FUNCTION_NAME, connection->io_path_acquired, timeout_milliseconds);
+
+  we_acquired = FALSE;
   
   if (connection->io_path_acquired)
     {
-      if (timeout_milliseconds != -1) 
-       res = dbus_condvar_wait_timeout (connection->io_path_cond,
-                                        connection->io_path_mutex,
-                                        timeout_milliseconds);
+      if (timeout_milliseconds != -1)
+        {
+          _dbus_verbose ("%s waiting %d for IO path to be acquirable\n",
+                         _DBUS_FUNCTION_NAME, timeout_milliseconds);
+          dbus_condvar_wait_timeout (connection->io_path_cond,
+                                     connection->io_path_mutex,
+                                     timeout_milliseconds);
+        }
       else
-       dbus_condvar_wait (connection->io_path_cond, connection->io_path_mutex);
+        {
+          while (connection->io_path_acquired)
+            {
+              _dbus_verbose ("%s waiting for IO path to be acquirable\n", _DBUS_FUNCTION_NAME);
+              dbus_condvar_wait (connection->io_path_cond, connection->io_path_mutex);
+            }
+        }
     }
   
-  if (res)
+  if (!connection->io_path_acquired)
     {
-      _dbus_assert (!connection->io_path_acquired);
-
+      we_acquired = TRUE;
       connection->io_path_acquired = TRUE;
     }
+  
+  _dbus_verbose ("%s end connection->io_path_acquired = %d we_acquired = %d\n",
+                 _DBUS_FUNCTION_NAME, connection->io_path_acquired, we_acquired);
+
+  _dbus_verbose ("%s unlocking io_path_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_unlock (connection->io_path_mutex);
 
-  _dbus_verbose ("%s end connection->io_path_acquired = %d res = %d\n",
-                 _DBUS_FUNCTION_NAME, connection->io_path_acquired, res);
+  CONNECTION_LOCK (connection);
   
-  return res;
+  HAVE_LOCK_CHECK (connection);
+
+  _dbus_connection_unref_unlocked (connection);
+  
+  return we_acquired;
 }
 
 /**
@@ -946,6 +977,11 @@ _dbus_connection_acquire_io_path (DBusConnection *connection,
 static void
 _dbus_connection_release_io_path (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
+  
+  _dbus_verbose ("%s locking io_path_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_lock (connection->io_path_mutex);
+  
   _dbus_assert (connection->io_path_acquired);
 
   _dbus_verbose ("%s start connection->io_path_acquired = %d\n",
@@ -953,8 +989,10 @@ _dbus_connection_release_io_path (DBusConnection *connection)
   
   connection->io_path_acquired = FALSE;
   dbus_condvar_wake_one (connection->io_path_cond);
-}
 
+  _dbus_verbose ("%s unlocking io_path_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_unlock (connection->io_path_mutex);
+}
 
 /**
  * Queues incoming messages and sends outgoing messages for this
@@ -999,11 +1037,15 @@ _dbus_connection_do_iteration_unlocked (DBusConnection *connection,
   if (_dbus_connection_acquire_io_path (connection,
                                        (flags & DBUS_ITERATION_BLOCK) ? timeout_milliseconds : 0))
     {
+      HAVE_LOCK_CHECK (connection);
+      
       _dbus_transport_do_iteration (connection->transport,
                                    flags, timeout_milliseconds);
       _dbus_connection_release_io_path (connection);
     }
 
+  HAVE_LOCK_CHECK (connection);
+
   _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
@@ -1298,8 +1340,10 @@ _dbus_connection_handle_watch (DBusWatch                   *watch,
   
   CONNECTION_LOCK (connection);
   _dbus_connection_acquire_io_path (connection, -1);
+  HAVE_LOCK_CHECK (connection);
   retval = _dbus_transport_handle_watch (connection->transport,
                                          watch, condition);
+
   _dbus_connection_release_io_path (connection);
 
   HAVE_LOCK_CHECK (connection);
@@ -2671,23 +2715,38 @@ dbus_connection_pop_message (DBusConnection *connection)
 }
 
 /**
- * Acquire the dispatcher. This must be done before dispatching
- * messages in order to guarantee the right order of
- * message delivery. May sleep and drop the connection mutex
- * while waiting for the dispatcher.
+ * Acquire the dispatcher. This is a separate lock so the main
+ * connection lock can be dropped to call out to application dispatch
+ * handlers.
  *
  * @param connection the connection.
  */
 static void
 _dbus_connection_acquire_dispatch (DBusConnection *connection)
 {
-  if (connection->dispatch_acquired)
+  HAVE_LOCK_CHECK (connection);
+
+  _dbus_connection_ref_unlocked (connection);
+  CONNECTION_UNLOCK (connection);
+  
+  _dbus_verbose ("%s locking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_lock (connection->dispatch_mutex);
+
+  while (connection->dispatch_acquired)
     {
+      _dbus_verbose ("%s waiting for dispatch to be acquirable\n", _DBUS_FUNCTION_NAME);
       dbus_condvar_wait (connection->dispatch_cond, connection->dispatch_mutex);
     }
+  
   _dbus_assert (!connection->dispatch_acquired);
 
   connection->dispatch_acquired = TRUE;
+
+  _dbus_verbose ("%s unlocking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_unlock (connection->dispatch_mutex);
+  
+  CONNECTION_LOCK (connection);
+  _dbus_connection_unref_unlocked (connection);
 }
 
 /**
@@ -2700,10 +2759,18 @@ _dbus_connection_acquire_dispatch (DBusConnection *connection)
 static void
 _dbus_connection_release_dispatch (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
+  
+  _dbus_verbose ("%s locking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_lock (connection->dispatch_mutex);
+  
   _dbus_assert (connection->dispatch_acquired);
 
   connection->dispatch_acquired = FALSE;
   dbus_condvar_wake_one (connection->dispatch_cond);
+
+  _dbus_verbose ("%s unlocking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_unlock (connection->dispatch_mutex);
 }
 
 static void
@@ -2893,7 +2960,8 @@ dbus_connection_dispatch (DBusConnection *connection)
   _dbus_connection_ref_unlocked (connection);
 
   _dbus_connection_acquire_dispatch (connection);
-  
+  HAVE_LOCK_CHECK (connection);
+
   /* This call may drop the lock during the execution (if waiting for
    * borrowed messages to be returned) but the order of message
    * dispatch if several threads call dispatch() is still
@@ -2940,6 +3008,7 @@ dbus_connection_dispatch (DBusConnection *connection)
   if (!_dbus_list_copy (&connection->filter_list, &filter_list_copy))
     {
       _dbus_connection_release_dispatch (connection);
+      HAVE_LOCK_CHECK (connection);
 
       _dbus_connection_failed_pop (connection, message_link);
 
@@ -3152,6 +3221,7 @@ dbus_connection_dispatch (DBusConnection *connection)
     }
   
   _dbus_connection_release_dispatch (connection);
+  HAVE_LOCK_CHECK (connection);
 
   _dbus_verbose ("%s before final status update\n", _DBUS_FUNCTION_NAME);
   status = _dbus_connection_get_dispatch_status_unlocked (connection);