* dbus/dbus-connection.c (_dbus_connection_open_internal): Fix

        bug 8780: the connection lock is only required while recording
        the shared connection, so protect only that code
        section. Don't require connection_lookup_shared to return a
        locked connection.

2 files changed, 29 insertions, 32 deletions

diff --git a/ChangeLog b/ChangeLog
index 3a26dc71..09e0ca33 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2006-10-26  Thiago Macieira  <thiago@kde.org>
 
+	* dbus/dbus-connection.c (_dbus_connection_open_internal): Fix
+        bug 8780: the connection lock is only required while recording
+        the shared connection, so protect only that code
+        section. Don't require connection_lookup_shared to return a
+        locked connection.
+
+2006-10-26  Thiago Macieira  <thiago@kde.org>
+
 	* tools/dbus-launch-x11.c (get_session_file, init_x_atoms):
         check if get_machine_uuid() returns NULL before proceeding any
         further: we can't init the X atoms or create a session file
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 80032bd1..7a20e047 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -1725,41 +1725,31 @@ _dbus_connection_open_internal (const char     *address,
           connection = connection_try_from_address_entry (entries[i],
                                                           &tmp_error);
 
-          if (connection != NULL)
+          if (connection != NULL && shared)
             {
-              CONNECTION_LOCK (connection);
-          
-              if (shared)
-                {
-                  const char *guid;
-                  
-                  connection->shareable = TRUE;
+              const char *guid;
                   
-                  /* guid may be NULL */
-                  guid = dbus_address_entry_get_value (entries[i], "guid");
+              connection->shareable = TRUE;
                   
-                  if (!connection_record_shared_unlocked (connection, guid))
-                    {
-                      _DBUS_SET_OOM (&tmp_error);
-                      _dbus_connection_close_possibly_shared_and_unlock (connection);
-                      dbus_connection_unref (connection);
-                      connection = NULL;
-                    }
+              /* guid may be NULL */
+              guid = dbus_address_entry_get_value (entries[i], "guid");
                   
-                  /* Note: as of now the connection is possibly shared
-                   * since another thread could have pulled it from the table.
-                   * However, we still have it locked so that thread isn't
-                   * doing anything more than blocking on the lock.
-                   */
+              CONNECTION_LOCK (connection);
+          
+              if (!connection_record_shared_unlocked (connection, guid))
+                {
+                  _DBUS_SET_OOM (&tmp_error);
+                  _dbus_connection_close_possibly_shared_and_unlock (connection);
+                  dbus_connection_unref (connection);
+                  connection = NULL;
                 }
+              else
+                CONNECTION_UNLOCK (connection);
             }
         }
       
       if (connection)
-        {
-          HAVE_LOCK_CHECK (connection);
-          break;
-        }
+        break;
 
       _DBUS_ASSERT_ERROR_IS_SET (&tmp_error);
       
@@ -1778,11 +1768,7 @@ _dbus_connection_open_internal (const char     *address,
       dbus_move_error (&first_error, error);
     }
   else
-    {
-      dbus_error_free (&first_error);
-
-      CONNECTION_UNLOCK (connection);
-    }
+    dbus_error_free (&first_error);
   
   dbus_address_entries_free (entries);
   return connection;
@@ -3344,7 +3330,10 @@ _dbus_connection_read_write_dispatch (DBusConnection *connection,
  * write, then read or write, then return.
  *
  * The way to think of this function is that it either makes some sort
- * of progress, or it blocks.
+ * of progress, or it blocks. Note that, while it is blocked on I/O, it
+ * cannot be interrupted (even by other threads), which makes this function
+ * unsuitable for applications that do more than just react to received
+ * messages.
  *
  * The return value indicates whether the disconnect message has been
  * processed, NOT whether the connection is connected. This is