diff options
Diffstat (limited to 'recipes/dbus/dbus-1.3.1/bugfix-17754.patch')
-rw-r--r-- | recipes/dbus/dbus-1.3.1/bugfix-17754.patch | 426 |
1 files changed, 426 insertions, 0 deletions
diff --git a/recipes/dbus/dbus-1.3.1/bugfix-17754.patch b/recipes/dbus/dbus-1.3.1/bugfix-17754.patch new file mode 100644 index 0000000000..f4568f377e --- /dev/null +++ b/recipes/dbus/dbus-1.3.1/bugfix-17754.patch @@ -0,0 +1,426 @@ +From 6ff1d079316cb730a54b4e0e95bd3e6e31f439de Mon Sep 17 00:00:00 2001 +From: Thiago Macieira <thiago@kde.org> +Date: Tue, 22 Jun 2010 13:13:23 +0000 +Subject: Fix the reentrancy issue reported on bug 17754. + +Patch based on patch by Havoc Pennington, with the references that +this is temporary removed. + + Patch based on one from Olivier Hochreutiner <olivier.hochreutiner + gmail.com> + + * dbus/dbus-connection.c (protected_change_timeout): remove the + elaborate nonworking hack to try to drop locks and just keep the + locks; this isn't right either, but at least is correct, though + it puts restrictions on apps. + + * dbus/dbus-connection.c (protected_change_watch): make the same + change as for timeouts + + * dbus/dbus-connection.c (dbus_connection_set_timeout_functions): + don't drop the lock here; add documentation of the problem to API + docs + (dbus_connection_set_watch_functions): same + + * dbus/dbus-connection.c (dbus_connection_get_data) + (dbus_connection_set_data): introduce a separate slot_mutex + protecting connection->slot_list so these two functions can be + called inside watch and timeout functions. Not sure this + is going to be a good idea. + + * dbus/dbus-connection.c (dbus_connection_unref) + (dbus_connection_ref): avoid using connection lock in ref/unref + so these can also be used in watch and timeout functions +--- +diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c +index 7f828c5..6f38d14 100644 +--- a/dbus/dbus-connection.c ++++ b/dbus/dbus-connection.c +@@ -75,6 +75,14 @@ + _dbus_mutex_unlock ((connection)->mutex); \ + } while (0) + ++#define SLOTS_LOCK(connection) do { \ ++ _dbus_mutex_lock ((connection)->slot_mutex); \ ++ } while (0) ++ ++#define SLOTS_UNLOCK(connection) do { \ ++ _dbus_mutex_unlock ((connection)->slot_mutex); \ ++ } while (0) ++ + #define DISPATCH_STATUS_NAME(s) \ + ((s) == DBUS_DISPATCH_COMPLETE ? "complete" : \ + (s) == DBUS_DISPATCH_DATA_REMAINS ? "data remains" : \ +@@ -263,6 +271,7 @@ struct DBusConnection + + DBusList *filter_list; /**< List of filters. */ + ++ DBusMutex *slot_mutex; /**< Lock on slot_list so overall connection lock need not be taken */ + DBusDataSlotList slot_list; /**< Data stored by allocated integer ID */ + + DBusHashTable *pending_replies; /**< Hash of message serials to #DBusPendingCall. */ +@@ -655,39 +664,42 @@ protected_change_watch (DBusConnection *connection, + DBusWatchToggleFunction toggle_function, + dbus_bool_t enabled) + { +- DBusWatchList *watches; + dbus_bool_t retval; +- ++ + HAVE_LOCK_CHECK (connection); + +- /* This isn't really safe or reasonable; a better pattern is the "do everything, then +- * drop lock and call out" one; but it has to be propagated up through all callers ++ /* The original purpose of protected_change_watch() was to hold a ++ * ref on the connection while dropping the connection lock, then ++ * calling out to the app. This was a broken hack that did not ++ * work, since the connection was in a hosed state (no WatchList ++ * field) while calling out. ++ * ++ * So for now we'll just keep the lock while calling out. This means ++ * apps are not allowed to call DBusConnection methods inside a ++ * watch function or they will deadlock. ++ * ++ * The "real fix" is to use the _and_unlock() pattern found ++ * elsewhere in the code, to defer calling out to the app until ++ * we're about to drop locks and return flow of control to the app ++ * anyway. ++ * ++ * See http://lists.freedesktop.org/archives/dbus/2007-July/thread.html#8144 + */ +- +- watches = connection->watches; +- if (watches) +- { +- connection->watches = NULL; +- _dbus_connection_ref_unlocked (connection); +- CONNECTION_UNLOCK (connection); + ++ if (connection->watches) ++ { + if (add_function) +- retval = (* add_function) (watches, watch); ++ retval = (* add_function) (connection->watches, watch); + else if (remove_function) + { + retval = TRUE; +- (* remove_function) (watches, watch); ++ (* remove_function) (connection->watches, watch); + } + else + { + retval = TRUE; +- (* toggle_function) (watches, watch, enabled); ++ (* toggle_function) (connection->watches, watch, enabled); + } +- +- CONNECTION_LOCK (connection); +- connection->watches = watches; +- _dbus_connection_unref_unlocked (connection); +- + return retval; + } + else +@@ -776,39 +788,42 @@ protected_change_timeout (DBusConnection *connection, + DBusTimeoutToggleFunction toggle_function, + dbus_bool_t enabled) + { +- DBusTimeoutList *timeouts; + dbus_bool_t retval; +- ++ + HAVE_LOCK_CHECK (connection); + +- /* This isn't really safe or reasonable; a better pattern is the "do everything, then +- * drop lock and call out" one; but it has to be propagated up through all callers ++ /* The original purpose of protected_change_timeout() was to hold a ++ * ref on the connection while dropping the connection lock, then ++ * calling out to the app. This was a broken hack that did not ++ * work, since the connection was in a hosed state (no TimeoutList ++ * field) while calling out. ++ * ++ * So for now we'll just keep the lock while calling out. This means ++ * apps are not allowed to call DBusConnection methods inside a ++ * timeout function or they will deadlock. ++ * ++ * The "real fix" is to use the _and_unlock() pattern found ++ * elsewhere in the code, to defer calling out to the app until ++ * we're about to drop locks and return flow of control to the app ++ * anyway. ++ * ++ * See http://lists.freedesktop.org/archives/dbus/2007-July/thread.html#8144 + */ +- +- timeouts = connection->timeouts; +- if (timeouts) +- { +- connection->timeouts = NULL; +- _dbus_connection_ref_unlocked (connection); +- CONNECTION_UNLOCK (connection); + ++ if (connection->timeouts) ++ { + if (add_function) +- retval = (* add_function) (timeouts, timeout); ++ retval = (* add_function) (connection->timeouts, timeout); + else if (remove_function) + { + retval = TRUE; +- (* remove_function) (timeouts, timeout); ++ (* remove_function) (connection->timeouts, timeout); + } + else + { + retval = TRUE; +- (* toggle_function) (timeouts, timeout, enabled); ++ (* toggle_function) (connection->timeouts, timeout, enabled); + } +- +- CONNECTION_LOCK (connection); +- connection->timeouts = timeouts; +- _dbus_connection_unref_unlocked (connection); +- + return retval; + } + else +@@ -1269,6 +1284,10 @@ _dbus_connection_new_for_transport (DBusTransport *transport) + if (connection->io_path_cond == NULL) + goto error; + ++ _dbus_mutex_new_at_location (&connection->slot_mutex); ++ if (connection->slot_mutex == NULL) ++ goto error; ++ + disconnect_message = dbus_message_new_signal (DBUS_PATH_LOCAL, + DBUS_INTERFACE_LOCAL, + "Disconnected"); +@@ -1345,6 +1364,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport) + _dbus_mutex_free_at_location (&connection->mutex); + _dbus_mutex_free_at_location (&connection->io_path_mutex); + _dbus_mutex_free_at_location (&connection->dispatch_mutex); ++ _dbus_mutex_free_at_location (&connection->slot_mutex); + dbus_free (connection); + } + if (pending_replies) +@@ -2618,9 +2638,14 @@ dbus_connection_ref (DBusConnection *connection) + + /* The connection lock is better than the global + * lock in the atomic increment fallback ++ * ++ * (FIXME but for now we always use the atomic version, ++ * to avoid taking the connection lock, due to ++ * the mess with set_timeout_functions()/set_watch_functions() ++ * calling out to the app without dropping locks) + */ + +-#ifdef DBUS_HAVE_ATOMIC_INT ++#if 1 + _dbus_atomic_inc (&connection->refcount); + #else + CONNECTION_LOCK (connection); +@@ -2732,6 +2757,8 @@ _dbus_connection_last_unref (DBusConnection *connection) + _dbus_mutex_free_at_location (&connection->io_path_mutex); + _dbus_mutex_free_at_location (&connection->dispatch_mutex); + ++ _dbus_mutex_free_at_location (&connection->slot_mutex); ++ + _dbus_mutex_free_at_location (&connection->mutex); + + dbus_free (connection); +@@ -2766,9 +2793,14 @@ dbus_connection_unref (DBusConnection *connection) + + /* The connection lock is better than the global + * lock in the atomic increment fallback ++ * ++ * (FIXME but for now we always use the atomic version, ++ * to avoid taking the connection lock, due to ++ * the mess with set_timeout_functions()/set_watch_functions() ++ * calling out to the app without dropping locks) + */ + +-#ifdef DBUS_HAVE_ATOMIC_INT ++#if 1 + last_unref = (_dbus_atomic_dec (&connection->refcount) == 1); + #else + CONNECTION_LOCK (connection); +@@ -4819,9 +4851,11 @@ dbus_connection_dispatch (DBusConnection *connection) + * should be that dbus_connection_set_watch_functions() has no effect, + * but the add_function and remove_function may have been called. + * +- * @todo We need to drop the lock when we call the +- * add/remove/toggled functions which can be a side effect +- * of setting the watch functions. ++ * @note The thread lock on DBusConnection is held while ++ * watch functions are invoked, so inside these functions you ++ * may not invoke any methods on DBusConnection or it will deadlock. ++ * See the comments in the code or http://lists.freedesktop.org/archives/dbus/2007-July/tread.html#8144 ++ * if you encounter this issue and want to attempt writing a patch. + * + * @param connection the connection. + * @param add_function function to begin monitoring a new descriptor. +@@ -4840,42 +4874,18 @@ dbus_connection_set_watch_functions (DBusConnection *connection, + DBusFreeFunction free_data_function) + { + dbus_bool_t retval; +- DBusWatchList *watches; + + _dbus_return_val_if_fail (connection != NULL, FALSE); + + CONNECTION_LOCK (connection); + +-#ifndef DBUS_DISABLE_CHECKS +- if (connection->watches == NULL) +- { +- _dbus_warn_check_failed ("Re-entrant call is not allowed\n"); +- return FALSE; +- } +-#endif +- +- /* ref connection for slightly better reentrancy */ +- _dbus_connection_ref_unlocked (connection); +- +- /* This can call back into user code, and we need to drop the +- * connection lock when it does. This is kind of a lame +- * way to do it. +- */ +- watches = connection->watches; +- connection->watches = NULL; +- CONNECTION_UNLOCK (connection); +- +- retval = _dbus_watch_list_set_functions (watches, ++ retval = _dbus_watch_list_set_functions (connection->watches, + add_function, remove_function, + toggled_function, + data, free_data_function); +- CONNECTION_LOCK (connection); +- connection->watches = watches; +- ++ + CONNECTION_UNLOCK (connection); +- /* drop our paranoid refcount */ +- dbus_connection_unref (connection); +- ++ + return retval; + } + +@@ -4904,6 +4914,12 @@ dbus_connection_set_watch_functions (DBusConnection *connection, + * given remove_function. The timer interval may change whenever the + * timeout is added, removed, or toggled. + * ++ * @note The thread lock on DBusConnection is held while ++ * timeout functions are invoked, so inside these functions you ++ * may not invoke any methods on DBusConnection or it will deadlock. ++ * See the comments in the code or http://lists.freedesktop.org/archives/dbus/2007-July/thread.html#8144 ++ * if you encounter this issue and want to attempt writing a patch. ++ * + * @param connection the connection. + * @param add_function function to add a timeout. + * @param remove_function function to remove a timeout. +@@ -4921,37 +4937,17 @@ dbus_connection_set_timeout_functions (DBusConnection *connection, + DBusFreeFunction free_data_function) + { + dbus_bool_t retval; +- DBusTimeoutList *timeouts; + + _dbus_return_val_if_fail (connection != NULL, FALSE); + + CONNECTION_LOCK (connection); + +-#ifndef DBUS_DISABLE_CHECKS +- if (connection->timeouts == NULL) +- { +- _dbus_warn_check_failed ("Re-entrant call is not allowed\n"); +- return FALSE; +- } +-#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; + } +@@ -5911,6 +5907,15 @@ dbus_connection_free_data_slot (dbus_int32_t *slot_p) + * the connection is finalized. The slot number + * must have been allocated with dbus_connection_allocate_data_slot(). + * ++ * @note This function does not take the ++ * main thread lock on DBusConnection, which allows it to be ++ * used from inside watch and timeout functions. (See the ++ * note in docs for dbus_connection_set_watch_functions().) ++ * A side effect of this is that you need to know there's ++ * a reference held on the connection while invoking ++ * dbus_connection_set_data(), or the connection could be ++ * finalized during dbus_connection_set_data(). ++ * + * @param connection the connection + * @param slot the slot number + * @param data the data to store +@@ -5930,14 +5935,14 @@ dbus_connection_set_data (DBusConnection *connection, + _dbus_return_val_if_fail (connection != NULL, FALSE); + _dbus_return_val_if_fail (slot >= 0, FALSE); + +- CONNECTION_LOCK (connection); ++ SLOTS_LOCK (connection); + + retval = _dbus_data_slot_list_set (&slot_allocator, + &connection->slot_list, + slot, data, free_data_func, + &old_free_func, &old_data); + +- CONNECTION_UNLOCK (connection); ++ SLOTS_UNLOCK (connection); + + if (retval) + { +@@ -5953,6 +5958,15 @@ dbus_connection_set_data (DBusConnection *connection, + * Retrieves data previously set with dbus_connection_set_data(). + * The slot must still be allocated (must not have been freed). + * ++ * @note This function does not take the ++ * main thread lock on DBusConnection, which allows it to be ++ * used from inside watch and timeout functions. (See the ++ * note in docs for dbus_connection_set_watch_functions().) ++ * A side effect of this is that you need to know there's ++ * a reference held on the connection while invoking ++ * dbus_connection_get_data(), or the connection could be ++ * finalized during dbus_connection_get_data(). ++ * + * @param connection the connection + * @param slot the slot to get data from + * @returns the data, or #NULL if not found +@@ -5965,13 +5979,13 @@ dbus_connection_get_data (DBusConnection *connection, + + _dbus_return_val_if_fail (connection != NULL, NULL); + +- CONNECTION_LOCK (connection); ++ SLOTS_LOCK (connection); + + res = _dbus_data_slot_list_get (&slot_allocator, + &connection->slot_list, + slot); + +- CONNECTION_UNLOCK (connection); ++ SLOTS_UNLOCK (connection); + + return res; + } +-- +cgit v0.8.3-6-g21f6 |