[prev in list] [next in list] [prev in thread] [next in thread]
List: freedesktop-dbus
Subject: Re: Out-of-memory error when using multiple threads
From: "Olivier Hochreutiner" <olivier.hochreutiner () gmail ! com>
Date: 2007-12-31 13:19:16
Message-ID: f967eaa30712310519n38eeded2y85ede1313ecf113d () mail ! gmail ! com
[Download RAW message or body]
> Just keeping the lock is probably an improvement over what we have now,
> so I'd take that patch.
Here is a patch doing that. I also modified
dbus_connection_set_timeout_functions() as it seemed to be broken too.
I tried to 'make check' and there is something broken, but also
without the patch (the error is different however). I am a bit
confused about the results, maybe you can try and see ?
> The ideal fix is probably rearranging the code a lot more to call out to
> the app in the right place (just before we'd be returning control to the
> app, like the other _and_unlock() functions), but it's a pretty big
> headache to do.
I don't have time do this right now but I'll write it somewhere on my
todo list...
Best,
Olivier
["timeout_race.patch" (text/x-patch)]
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 0de5f22..b2ef1bb 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -777,13 +777,13 @@ protected_change_timeout (DBusConnection *connection,
* drop lock and call out" one; but it has to be propagated up through all callers
*/
- timeouts = connection->timeouts;
- if (timeouts)
+ if (connection->timeouts)
{
- connection->timeouts = NULL;
- _dbus_connection_ref_unlocked (connection);
- CONNECTION_UNLOCK (connection);
-
+ /* Here we don't drop the connection lock to avoid that several threads try to use
+ * the timeouts field at the same time. This however forbids any call to dbus
+ * functions dealing with the connection in {add,remove,toogle}_function otherwise
+ * it would deadlock.
+ */
if (add_function)
retval = (* add_function) (timeouts, timeout);
else if (remove_function)
@@ -797,10 +797,6 @@ protected_change_timeout (DBusConnection *connection,
(* toggle_function) (timeouts, timeout, enabled);
}
- CONNECTION_LOCK (connection);
- connection->timeouts = timeouts;
- _dbus_connection_unref_unlocked (connection);
-
return retval;
}
else
@@ -4668,23 +4664,12 @@ dbus_connection_set_timeout_functions (DBusConnection *connection,
}
#endif
- /* ref connection for slightly better reentrancy */
- _dbus_connection_ref_unlocked (connection);
-
- timeouts = connection->timeouts;
- connection->timeouts = NULL;
- CONNECTION_UNLOCK (connection);
-
- retval = _dbus_timeout_list_set_functions (timeouts,
+ retval = _dbus_timeout_list_set_functions (connection->timeouts,
add_function, remove_function,
toggled_function,
data, free_data_function);
- CONNECTION_LOCK (connection);
- connection->timeouts = timeouts;
CONNECTION_UNLOCK (connection);
- /* drop our paranoid refcount */
- dbus_connection_unref (connection);
return retval;
}
_______________________________________________
dbus mailing list
dbus@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dbus
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic