diff options
Diffstat (limited to 'abs/core/dbus-core/bug17754.patch')
-rw-r--r-- | abs/core/dbus-core/bug17754.patch | 392 |
1 files changed, 392 insertions, 0 deletions
diff --git a/abs/core/dbus-core/bug17754.patch b/abs/core/dbus-core/bug17754.patch new file mode 100644 index 0000000..7f677ac --- /dev/null +++ b/abs/core/dbus-core/bug17754.patch @@ -0,0 +1,392 @@ +diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c +index b3cfa3d..e6d261f 100644 +--- a/dbus/dbus-connection.c ++++ b/dbus/dbus-connection.c +@@ -73,6 +73,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" : \ +@@ -257,6 +265,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. */ +@@ -649,39 +658,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 +@@ -770,39 +782,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 +@@ -1263,6 +1278,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"); +@@ -1339,6 +1358,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) +@@ -2612,9 +2632,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); +@@ -2726,6 +2751,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); +@@ -2760,9 +2787,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); +@@ -4708,9 +4740,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. +@@ -4729,43 +4763,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 to %s is not allowed\n", +- _DBUS_FUNCTION_NAME); +- 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; + } + +@@ -4794,6 +4803,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. +@@ -4811,38 +4826,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 to %s is not allowed\n", +- _DBUS_FUNCTION_NAME); +- 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; + } +@@ -5805,6 +5799,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 +@@ -5824,14 +5827,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) + { +@@ -5847,6 +5850,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 +@@ -5859,13 +5871,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; + } |