[patch] Fix locking for _dbus_connection_queue_received_message_link

[Timo Teräs]

Hi,

I've bumped into locking problem when experimenting with multithreaded 
D-BUS program.

I got a dead lock with following backtrace:
#8  0xb7f855cf in pthread_mutex_lock () from /lib/tls/libc.so.6
#9  0xb7c307af in dbus_gmutex_lock (mutex=0xfffffffc) at dbus-gthread.c:96
#10 0xb7c11a96 in dbus_mutex_lock (mutex=0xb7a78b18) at dbus-threads.c:91
#11 0xb7bff986 in _dbus_connection_remove_timeout_locked (
     connection=0xffffffff, timeout=0xfffffffc) at dbus-connection.c:565
#12 0xb7bff6b4 in _dbus_connection_queue_received_message_link (
     connection=0x8060a20, link=0x8091520) at dbus-connection.c:345
#13 0xb7c12b60 in _dbus_transport_queue_messages (transport=0x805f4a8)
     at dbus-transport.c:868
#14 0xb7c136c1 in do_reading (transport=0x805f4a8) at 
dbus-transport-unix.c:689
#15 0xb7c1381e in unix_handle_watch (transport=0x805f4a8, watch=0x0,
     flags=3081210648) at dbus-transport-unix.c:743
#16 0xb7c126a0 in _dbus_transport_handle_watch (transport=0x805f4a8,
     watch=0x805f550, condition=1) at dbus-transport.c:580
#17 0xb7c001da in _dbus_connection_handle_watch (watch=0xfffffffc,
     condition=4294967292, data=0x8060a20) at dbus-connection.c:1060
#18 0xb7c1448c in dbus_watch_handle (watch=0x805f550, flags=1)
     at dbus-watch.c:595
#19 0xb7c2c577 in dbus_gsource_dispatch (source=0xfffffffc, callback=0,
     user_data=0x0, is_server=0) at dbus-gmain.c:232
#20 0xb7c2c5fe in gsource_connection_dispatch (source=0x8062038,
     callback=0xfffffffc, user_data=0xfffffffc) at dbus-gmain.c:254
#21 0xb7b9c902 in g_main_depth () from /usr/lib/libglib-2.0.so.0
#22 0xb7b9d9f8 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#23 0xb7b9dd30 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#24 0xb7b9e373 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0

I also added some debugging prints and realized that CONNECTION_LOCK is 
called twice in this code path. The connection lock is first acquired in 
_dbus_connection_handle_watch().The second time comes when 
_dbus_connection_queue_received_message_link() calls 
_dbus_connection_remove_timeout_locked() which tries to also acquire the 
connection lock and deadlocks (GMutexes aren't recursive).

If I'm not mistaken the _dbus_connection_queue_received_message_link() 
will always be called from the _dbus_connection_handle_watch() so the 
connection lock is always held when entering that function. So, I 
changed the call from _dbus_connection_remove_timeout_locked() to 
_dbus_connection_remove_timeout(). Patch attached.

It also seems this was the only place where the _remove_timeout_locked 
was called so it is not used anymore. The second version of patch 
includes also removal of that function.

Cheers,
   Timo
-------------- next part --------------
Index: dbus/dbus-connection.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection.c,v
retrieving revision 1.86
diff -u -r1.86 dbus-connection.c
--- dbus/dbus-connection.c	27 Nov 2004 07:30:22 -0000	1.86
+++ dbus/dbus-connection.c	11 Jan 2005 12:45:18 -0000
@@ -224,8 +224,6 @@
 #endif 
 };
 
-static void               _dbus_connection_remove_timeout_locked             (DBusConnection     *connection,
-                                                                              DBusTimeout        *timeout);
 static DBusDispatchStatus _dbus_connection_get_dispatch_status_unlocked      (DBusConnection     *connection);
 static void               _dbus_connection_update_dispatch_status_and_unlock (DBusConnection     *connection,
                                                                               DBusDispatchStatus  new_status);
@@ -349,8 +347,8 @@
       if (pending != NULL)
 	{
 	  if (pending->timeout_added)
-	    _dbus_connection_remove_timeout_locked (connection,
-                                                    pending->timeout);
+	    _dbus_connection_remove_timeout (connection,
+                                             pending->timeout);
 
 	  pending->timeout_added = FALSE;
 	}
@@ -594,15 +592,6 @@
 				       timeout);
 }
 
-static void
-_dbus_connection_remove_timeout_locked (DBusConnection *connection,
-					DBusTimeout    *timeout)
-{
-  CONNECTION_LOCK (connection);
-  _dbus_connection_remove_timeout (connection, timeout);
-  CONNECTION_UNLOCK (connection);
-}
-
 /**
  * Toggles a timeout and notifies app via connection's
  * DBusTimeoutToggledFunction if available. It's an error to call this
-------------- next part --------------
Index: dbus/dbus-connection.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection.c,v
retrieving revision 1.86
diff -u -r1.86 dbus-connection.c
--- dbus/dbus-connection.c	27 Nov 2004 07:30:22 -0000	1.86
+++ dbus/dbus-connection.c	11 Jan 2005 12:31:59 -0000
@@ -349,8 +349,8 @@
       if (pending != NULL)
 	{
 	  if (pending->timeout_added)
-	    _dbus_connection_remove_timeout_locked (connection,
-                                                    pending->timeout);
+	    _dbus_connection_remove_timeout (connection,
+                                             pending->timeout);
 
 	  pending->timeout_added = FALSE;
 	}