summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat (limited to 'sys-apps/dbus/files/multithread-locking-fix.patch')
-rw-r--r--sys-apps/dbus/files/multithread-locking-fix.patch455
1 files changed, 0 insertions, 455 deletions
diff --git a/sys-apps/dbus/files/multithread-locking-fix.patch b/sys-apps/dbus/files/multithread-locking-fix.patch
deleted file mode 100644
index 743c398..0000000
--- a/sys-apps/dbus/files/multithread-locking-fix.patch
+++ /dev/null
@@ -1,455 +0,0 @@
-2008-01-01 Havoc Pennington <hp at redhat.com>
-
- 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
----
- ChangeLog | 28 +++++++
- dbus/dbus-connection.c | 196 ++++++++++++++++++++++++++----------------------
- 2 files changed, 134 insertions(+), 90 deletions(-)
-
-diff --git a/ChangeLog b/ChangeLog
-index 35e35a7..ec761f3 100644
---- a/ChangeLog
-+++ b/ChangeLog
-@@ -1,3 +1,31 @@
-+2008-01-01 Havoc Pennington <hp at redhat.com>
-+
-+ 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
-+
- 2008-04-16 John (J5) Palmieri <johnp@redhat.com>
-
- * tools/dbus-monitor.c: take out the trailing comma in the
-diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
-index 0de5f22..177fb2e 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. */
-@@ -647,38 +656,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)
-+ if (connection->watches)
- {
-- connection->watches = NULL;
-- _dbus_connection_ref_unlocked (connection);
-- CONNECTION_UNLOCK (connection);
--
- 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;
- }
-@@ -768,39 +781,43 @@ 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)
-+ if (connection->timeouts)
- {
-- connection->timeouts = NULL;
-- _dbus_connection_ref_unlocked (connection);
-- CONNECTION_UNLOCK (connection);
--
- 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
-@@ -1219,6 +1236,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");
-@@ -1295,6 +1316,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)
-@@ -2467,9 +2489,15 @@ 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
-+ /* #ifdef DBUS_HAVE_ATOMIC_INT */
-+#if 1
- _dbus_atomic_inc (&connection->refcount);
- #else
- CONNECTION_LOCK (connection);
-@@ -2581,6 +2609,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);
-@@ -2615,9 +2645,15 @@ 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
-+ /* #ifdef DBUS_HAVE_ATOMIC_INT */
-+#if 1
- last_unref = (_dbus_atomic_dec (&connection->refcount) == 1);
- #else
- CONNECTION_LOCK (connection);
-@@ -4550,10 +4586,12 @@ 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 For now, 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/thread.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.
- * @param remove_function function to stop monitoring a descriptor.
-@@ -4571,42 +4609,17 @@ 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;
- }
-@@ -4636,6 +4649,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 For now, 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.
-@@ -4653,38 +4672,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;
- }
-@@ -5612,6 +5610,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 As a temporary hack, 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
-@@ -5631,14 +5638,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)
- {
-@@ -5654,6 +5661,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 As a temporary hack, 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
-@@ -5666,13 +5682,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;
- }