2003-02-20 Alexander Larsson <alexl@redhat.com>

	* dbus/dbus-transport-unix.c (unix_do_iteration):
	Unlock the connection mutex during a blocking select call.
	Add todo about how we need a way to wake up the select.

	* dbus/dbus-connection-internal.h:
	* dbus/dbus-connection.c:
	Add _dbus_connection_lock and _dbus_connection_unlock.

4 files changed, 62 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 4404bdf3..070196df 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2003-02-20  Alexander Larsson  <alexl@redhat.com>
+
+	* dbus/dbus-transport-unix.c (unix_do_iteration):
+	Unlock the connection mutex during a blocking select call.
+	Add todo about how we need a way to wake up the select.
+
+	* dbus/dbus-connection-internal.h: 
+	* dbus/dbus-connection.c:
+	Add _dbus_connection_lock and _dbus_connection_unlock.
+
 2003-02-19  Havoc Pennington  <hp@pobox.com>
 
 	* Doxyfile.in (PREDEFINED): put DOXYGEN_SHOULD_SKIP_THIS in
diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h
index 0606d1bd..64c4cf39 100644
--- a/dbus/dbus-connection-internal.h
+++ b/dbus/dbus-connection-internal.h
@@ -38,6 +38,9 @@ typedef enum
   DBUS_ITERATION_BLOCK      = 1 << 2  /**< Block if nothing to do. */
 } DBusIterationFlags;
 
+void             _dbus_connection_lock                  (DBusConnection *connection);
+void             _dbus_connection_unlock                (DBusConnection *connection);
+
 void             _dbus_connection_ref_unlocked          (DBusConnection *connection);
 
 dbus_bool_t     _dbus_connection_queue_received_message (DBusConnection *connection,
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 7da89bdc..64e83d11 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -134,6 +134,27 @@ static void _dbus_connection_free_data_slots_nolock (DBusConnection *connection)
 static void _dbus_connection_remove_timeout_locked (DBusConnection *connection,
 						    DBusTimeout    *timeout);
 
+/**
+ * Acquires the connection lock.
+ *
+ * @param connection the connection.
+ */
+void
+_dbus_connection_lock (DBusConnection *connection)
+{
+  dbus_mutex_lock (connection->mutex);
+}
+
+/**
+ * Releases the connection lock.
+ *
+ * @param connection the connection.
+ */
+void
+_dbus_connection_unlock (DBusConnection *connection)
+{
+  dbus_mutex_unlock (connection->mutex);
+}
 
 
 /**
@@ -336,9 +357,9 @@ static void
 _dbus_connection_remove_timeout_locked (DBusConnection *connection,
 					DBusTimeout    *timeout)
 {
-    dbus_mutex_lock (connection->mutex);
-    _dbus_connection_remove_timeout (connection, timeout);
-    dbus_mutex_unlock (connection->mutex);
+  dbus_mutex_lock (connection->mutex);
+  _dbus_connection_remove_timeout (connection, timeout);
+  dbus_mutex_unlock (connection->mutex);
 }
 
 
diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c
index c5dce8d7..b30dc1d6 100644
--- a/dbus/dbus-transport-unix.c
+++ b/dbus/dbus-transport-unix.c
@@ -884,6 +884,13 @@ unix_messages_pending (DBusTransport *transport,
 }
 
 /* FIXME use _dbus_poll(), not select() */
+/**
+ * @todo We need to have a way to wake up the select sleep if
+ * a new iteration request comes in with a flag (read/write) that
+ * we're not currently serving. Otherwise a call that just reads
+ * could block a write call forever (if there are no incoming
+ * messages).
+ */
 static  void
 unix_do_iteration (DBusTransport *transport,
                    unsigned int   flags,
@@ -893,6 +900,7 @@ unix_do_iteration (DBusTransport *transport,
   fd_set read_set;
   fd_set write_set;
   dbus_bool_t do_select;
+  int select_res;
 
   _dbus_verbose (" iteration flags = %s%s timeout = %d\n",
                  flags & DBUS_ITERATION_DO_READING ? "read" : "",
@@ -984,9 +992,24 @@ unix_do_iteration (DBusTransport *transport,
           timeout.tv_usec = 0;
           use_timeout = TRUE;
         }
+
+      /* For blocking selects we drop the connection lock here
+       * to avoid blocking out connection access during a potentially
+       * indefinite blocking call. The io path is still protected
+       * by the io_path_cond condvar, so we won't reenter this.
+       */
+      if (flags & DBUS_ITERATION_BLOCK)
+	_dbus_connection_unlock (transport->connection);
+      
+      select_res = select (unix_transport->fd + 1,
+			   &read_set, &write_set, &err_set,
+			   use_timeout ? &timeout : NULL);
+
+      if (flags & DBUS_ITERATION_BLOCK)
+	_dbus_connection_lock (transport->connection);
+      
       
-      if (select (unix_transport->fd + 1, &read_set, &write_set, &err_set,
-                  use_timeout ? &timeout : NULL) >= 0)
+      if (select_res >= 0)
         {
           if (FD_ISSET (unix_transport->fd, &err_set))
             do_io_error (transport);