dbus/dbus dbus-connection-internal.h, 1.22, 1.23 dbus-connection.c,
1.93, 1.94 dbus-object-tree.c, 1.13,
1.14 dbus-server-protected.h, 1.14, 1.15 dbus-server-unix.c,
1.25, 1.26 dbus-server.c, 1.36, 1.37 dbus-transport-unix.c,
1.44, 1.45 dbus-transport.c, 1.43, 1.44
Havoc Pennington
hp at freedesktop.org
Sun Feb 13 09:16:27 PST 2005
Update of /cvs/dbus/dbus/dbus
In directory gabe:/tmp/cvs-serv26019/dbus
Modified Files:
dbus-connection-internal.h dbus-connection.c
dbus-object-tree.c dbus-server-protected.h dbus-server-unix.c
dbus-server.c dbus-transport-unix.c dbus-transport.c
Log Message:
2005-02-13 Havoc Pennington <hp at redhat.com>
* dbus/dbus-object-tree.c (handle_default_introspect_and_unlock):
fix a double-unlock
* dbus/dbus-connection.c
(_dbus_connection_detach_pending_call_unlocked): add this
Initial semi-correct pass through to fix thread locking; there are
still some issues with the condition variable paths I'm pretty
sure
* dbus/dbus-server.c: add a mutex on DBusServer and appropriate
lock/unlock calls
* dbus/dbus-connection.c (_dbus_connection_do_iteration_unlocked):
rename to add _unlocked
(struct DBusConnection): move "dispatch_acquired" and
"io_path_acquired" to use only one bit each.
(CONNECTION_LOCK, CONNECTION_UNLOCK): add checks with !DBUS_DISABLE_CHECKS
(dbus_connection_set_watch_functions): hacky fix to reentrancy
(_dbus_connection_add_watch, _dbus_connection_remove_watch)
(_dbus_connection_toggle_watch, _dbus_connection_add_timeout)
(_dbus_connection_remove_timeout)
(_dbus_connection_toggle_timeout): drop lock when calling out to
user functions; done in a hacky/bad way.
(_dbus_connection_send_and_unlock): add a missing unlock
(_dbus_connection_block_for_reply): add a missing unlock
* dbus/dbus-transport.c (_dbus_transport_get_is_authenticated):
drop lock in a hacky probably unsafe way to call out to user
function
Index: dbus-connection-internal.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection-internal.h,v
retrieving revision 1.22
retrieving revision 1.23
diff -u -d -r1.22 -r1.23
--- dbus-connection-internal.h 10 Feb 2005 23:47:54 -0000 1.22
+++ dbus-connection-internal.h 13 Feb 2005 17:16:25 -0000 1.23
@@ -74,7 +74,7 @@
DBusTimeout *timeout,
dbus_bool_t enabled);
DBusConnection* _dbus_connection_new_for_transport (DBusTransport *transport);
-void _dbus_connection_do_iteration (DBusConnection *connection,
+void _dbus_connection_do_iteration_unlocked (DBusConnection *connection,
unsigned int flags,
int timeout_milliseconds);
Index: dbus-connection.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection.c,v
retrieving revision 1.93
retrieving revision 1.94
diff -u -d -r1.93 -r1.94
--- dbus-connection.c 10 Feb 2005 23:47:54 -0000 1.93
+++ dbus-connection.c 13 Feb 2005 17:16:25 -0000 1.94
@@ -1,7 +1,7 @@
/* -*- mode: C; c-file-style: "gnu" -*- */
/* dbus-connection.c DBusConnection object
*
- * Copyright (C) 2002, 2003, 2004 Red Hat Inc.
+ * Copyright (C) 2002, 2003, 2004, 2005 Red Hat Inc.
*
* Licensed under the Academic Free License version 2.1
*
@@ -39,20 +39,37 @@
#include "dbus-pending-call.h"
#include "dbus-object-tree.h"
-#if 0
-#define CONNECTION_LOCK(connection) do { \
- _dbus_verbose (" LOCK: %s\n", _DBUS_FUNCTION_NAME); \
- dbus_mutex_lock ((connection)->mutex); \
+#ifdef DBUS_DISABLE_CHECKS
+#define TOOK_LOCK_CHECK(connection)
+#define RELEASING_LOCK_CHECK(connection)
+#define HAVE_LOCK_CHECK(connection)
+#else
+#define TOOK_LOCK_CHECK(connection) do { \
+ _dbus_assert (!(connection)->have_connection_lock); \
+ (connection)->have_connection_lock = TRUE; \
} while (0)
-#define CONNECTION_UNLOCK(connection) do { \
- _dbus_verbose (" UNLOCK: %s\n", _DBUS_FUNCTION_NAME); \
- dbus_mutex_unlock ((connection)->mutex); \
+#define RELEASING_LOCK_CHECK(connection) do { \
+ _dbus_assert ((connection)->have_connection_lock); \
+ (connection)->have_connection_lock = FALSE; \
} while (0)
-#else
-#define CONNECTION_LOCK(connection) dbus_mutex_lock ((connection)->mutex)
-#define CONNECTION_UNLOCK(connection) dbus_mutex_unlock ((connection)->mutex)
+#define HAVE_LOCK_CHECK(connection) _dbus_assert ((connection)->have_connection_lock)
+/* A "DO_NOT_HAVE_LOCK_CHECK" is impossible since we need the lock to check the flag */
#endif
+#define TRACE_LOCKS 1
+
+#define CONNECTION_LOCK(connection) do { \
+ if (TRACE_LOCKS) { _dbus_verbose (" LOCK: %s\n", _DBUS_FUNCTION_NAME); } \
+ dbus_mutex_lock ((connection)->mutex); \
+ TOOK_LOCK_CHECK (connection); \
+ } while (0)
+
+#define CONNECTION_UNLOCK(connection) do { \
+ if (TRACE_LOCKS) { _dbus_verbose (" UNLOCK: %s\n", _DBUS_FUNCTION_NAME); } \
+ RELEASING_LOCK_CHECK (connection); \
+ dbus_mutex_unlock ((connection)->mutex); \
+ } while (0)
+
#define DISPATCH_STATUS_NAME(s) \
((s) == DBUS_DISPATCH_COMPLETE ? "complete" : \
(s) == DBUS_DISPATCH_DATA_REMAINS ? "data remains" : \
@@ -171,10 +188,7 @@
DBusMutex *mutex; /**< Lock on the entire DBusConnection */
- dbus_bool_t dispatch_acquired; /**< Protects dispatch() */
DBusCondVar *dispatch_cond; /**< Protects dispatch() */
-
- dbus_bool_t io_path_acquired; /**< Protects transport io path */
DBusCondVar *io_path_cond; /**< Protects transport io path */
DBusList *outgoing_messages; /**< Queue of messages we need to send, send the end of the list first. */
@@ -215,9 +229,16 @@
* for the global linked list mempool lock
*/
DBusObjectTree *objects; /**< Object path handlers registered with this connection */
-
+
+ unsigned int dispatch_acquired : 1; /**< Someone has dispatch path */
+ unsigned int io_path_acquired : 1; /**< Someone has transport io path */
+
unsigned int exit_on_disconnect : 1; /**< If #TRUE, exit after handling disconnect signal */
-
+
+#ifndef DBUS_DISABLE_CHECKS
+ unsigned int have_connection_lock : 1; /**< Used to check locking */
+#endif
+
#ifndef DBUS_DISABLE_CHECKS
int generation; /**< _dbus_current_generation that should correspond to this connection */
#endif
@@ -387,6 +408,8 @@
_dbus_connection_queue_synthesized_message_link (DBusConnection *connection,
DBusList *link)
{
+ HAVE_LOCK_CHECK (connection);
+
_dbus_list_append_link (&connection->incoming_messages, link);
connection->n_incoming += 1;
@@ -408,6 +431,7 @@
dbus_bool_t
_dbus_connection_has_messages_to_send_unlocked (DBusConnection *connection)
{
+ HAVE_LOCK_CHECK (connection);
return connection->outgoing_messages != NULL;
}
@@ -441,6 +465,8 @@
DBusMessage*
_dbus_connection_get_message_to_send (DBusConnection *connection)
{
+ HAVE_LOCK_CHECK (connection);
+
return _dbus_list_get_last (&connection->outgoing_messages);
}
@@ -458,6 +484,8 @@
{
DBusList *link;
+ HAVE_LOCK_CHECK (connection);
+
/* This can be called before we even complete authentication, since
* it's called on disconnect to clean up the outgoing queue.
* It's also called as we successfully send each message.
@@ -495,6 +523,62 @@
dbus_message_unref (message);
}
+typedef dbus_bool_t (* DBusWatchAddFunction) (DBusWatchList *list,
+ DBusWatch *watch);
+typedef void (* DBusWatchRemoveFunction) (DBusWatchList *list,
+ DBusWatch *watch);
+typedef void (* DBusWatchToggleFunction) (DBusWatchList *list,
+ DBusWatch *watch,
+ dbus_bool_t enabled);
+
+static dbus_bool_t
+protected_change_watch (DBusConnection *connection,
+ DBusWatch *watch,
+ DBusWatchAddFunction add_function,
+ DBusWatchRemoveFunction remove_function,
+ 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
+ */
+
+ watches = connection->watches;
+ if (watches)
+ {
+ connection->watches = NULL;
+ _dbus_connection_ref_unlocked (connection);
+ CONNECTION_UNLOCK (connection);
+
+ if (add_function)
+ retval = (* add_function) (watches, watch);
+ else if (remove_function)
+ {
+ retval = TRUE;
+ (* remove_function) (watches, watch);
+ }
+ else
+ {
+ retval = TRUE;
+ (* toggle_function) (watches, watch, enabled);
+ }
+
+ CONNECTION_LOCK (connection);
+ connection->watches = watches;
+ _dbus_connection_unref_unlocked (connection);
+
+ return retval;
+ }
+ else
+ return FALSE;
+}
+
+
/**
* Adds a watch using the connection's DBusAddWatchFunction if
* available. Otherwise records the watch to be added when said
@@ -509,11 +593,9 @@
_dbus_connection_add_watch (DBusConnection *connection,
DBusWatch *watch)
{
- if (connection->watches) /* null during finalize */
- return _dbus_watch_list_add_watch (connection->watches,
- watch);
- else
- return FALSE;
+ return protected_change_watch (connection, watch,
+ _dbus_watch_list_add_watch,
+ NULL, NULL, FALSE);
}
/**
@@ -528,9 +610,10 @@
_dbus_connection_remove_watch (DBusConnection *connection,
DBusWatch *watch)
{
- if (connection->watches) /* null during finalize */
- _dbus_watch_list_remove_watch (connection->watches,
- watch);
+ protected_change_watch (connection, watch,
+ NULL,
+ _dbus_watch_list_remove_watch,
+ NULL, FALSE);
}
/**
@@ -549,10 +632,66 @@
dbus_bool_t enabled)
{
_dbus_assert (watch != NULL);
+
+ protected_change_watch (connection, watch,
+ NULL, NULL,
+ _dbus_watch_list_toggle_watch,
+ enabled);
+}
+
+typedef dbus_bool_t (* DBusTimeoutAddFunction) (DBusTimeoutList *list,
+ DBusTimeout *timeout);
+typedef void (* DBusTimeoutRemoveFunction) (DBusTimeoutList *list,
+ DBusTimeout *timeout);
+typedef void (* DBusTimeoutToggleFunction) (DBusTimeoutList *list,
+ DBusTimeout *timeout,
+ dbus_bool_t enabled);
+
+static dbus_bool_t
+protected_change_timeout (DBusConnection *connection,
+ DBusTimeout *timeout,
+ DBusTimeoutAddFunction add_function,
+ DBusTimeoutRemoveFunction remove_function,
+ DBusTimeoutToggleFunction toggle_function,
+ dbus_bool_t enabled)
+{
+ DBusTimeoutList *timeouts;
+ dbus_bool_t retval;
- if (connection->watches) /* null during finalize */
- _dbus_watch_list_toggle_watch (connection->watches,
- watch, enabled);
+ 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
+ */
+
+ timeouts = connection->timeouts;
+ if (timeouts)
+ {
+ connection->timeouts = NULL;
+ _dbus_connection_ref_unlocked (connection);
+ CONNECTION_UNLOCK (connection);
+
+ if (add_function)
+ retval = (* add_function) (timeouts, timeout);
+ else if (remove_function)
+ {
+ retval = TRUE;
+ (* remove_function) (timeouts, timeout);
+ }
+ else
+ {
+ retval = TRUE;
+ (* toggle_function) (timeouts, timeout, enabled);
+ }
+
+ CONNECTION_LOCK (connection);
+ connection->timeouts = timeouts;
+ _dbus_connection_unref_unlocked (connection);
+
+ return retval;
+ }
+ else
+ return FALSE;
}
/**
@@ -570,11 +709,9 @@
_dbus_connection_add_timeout (DBusConnection *connection,
DBusTimeout *timeout)
{
- if (connection->timeouts) /* null during finalize */
- return _dbus_timeout_list_add_timeout (connection->timeouts,
- timeout);
- else
- return FALSE;
+ return protected_change_timeout (connection, timeout,
+ _dbus_timeout_list_add_timeout,
+ NULL, NULL, FALSE);
}
/**
@@ -589,9 +726,10 @@
_dbus_connection_remove_timeout (DBusConnection *connection,
DBusTimeout *timeout)
{
- if (connection->timeouts) /* null during finalize */
- _dbus_timeout_list_remove_timeout (connection->timeouts,
- timeout);
+ protected_change_timeout (connection, timeout,
+ NULL,
+ _dbus_timeout_list_remove_timeout,
+ NULL, FALSE);
}
/**
@@ -604,19 +742,22 @@
* @param enabled whether to enable or disable
*/
void
-_dbus_connection_toggle_timeout (DBusConnection *connection,
+_dbus_connection_toggle_timeout (DBusConnection *connection,
DBusTimeout *timeout,
- dbus_bool_t enabled)
+ dbus_bool_t enabled)
{
- if (connection->timeouts) /* null during finalize */
- _dbus_timeout_list_toggle_timeout (connection->timeouts,
- timeout, enabled);
+ protected_change_timeout (connection, timeout,
+ NULL, NULL,
+ _dbus_timeout_list_toggle_timeout,
+ enabled);
}
static dbus_bool_t
_dbus_connection_attach_pending_call_unlocked (DBusConnection *connection,
DBusPendingCall *pending)
{
+ HAVE_LOCK_CHECK (connection);
+
_dbus_assert (pending->reply_serial != 0);
if (!_dbus_connection_add_timeout (connection, pending->timeout))
@@ -627,6 +768,8 @@
pending))
{
_dbus_connection_remove_timeout (connection, pending->timeout);
+
+ HAVE_LOCK_CHECK (connection);
return FALSE;
}
@@ -634,6 +777,8 @@
pending->connection = connection;
dbus_pending_call_ref (pending);
+
+ HAVE_LOCK_CHECK (connection);
return TRUE;
}
@@ -664,6 +809,19 @@
}
static void
+_dbus_connection_detach_pending_call_unlocked (DBusConnection *connection,
+ DBusPendingCall *pending)
+{
+ /* Can't have a destroy notifier on the pending call if we're going to do this */
+
+ dbus_pending_call_ref (pending);
+ _dbus_hash_table_remove_int (connection->pending_replies,
+ pending->reply_serial);
+ _dbus_assert (pending->connection == NULL);
+ dbus_pending_call_unref (pending);
+}
+
+static void
_dbus_connection_detach_pending_call_and_unlock (DBusConnection *connection,
DBusPendingCall *pending)
{
@@ -674,6 +832,7 @@
dbus_pending_call_ref (pending);
_dbus_hash_table_remove_int (connection->pending_replies,
pending->reply_serial);
+ _dbus_assert (pending->connection == NULL);
CONNECTION_UNLOCK (connection);
dbus_pending_call_unref (pending);
}
@@ -749,14 +908,25 @@
{
dbus_bool_t res = TRUE;
+ _dbus_verbose ("%s start connection->io_path_acquired = %d timeout = %d\n",
+ _DBUS_FUNCTION_NAME, connection->io_path_acquired, timeout_milliseconds);
+
if (connection->io_path_acquired)
{
+#ifndef DBUS_DISABLE_CHECKS
+ connection->have_connection_lock = FALSE;
+#endif
+
if (timeout_milliseconds != -1)
res = dbus_condvar_wait_timeout (connection->io_path_cond,
connection->mutex,
timeout_milliseconds);
else
dbus_condvar_wait (connection->io_path_cond, connection->mutex);
+
+#ifndef DBUS_DISABLE_CHECKS
+ connection->have_connection_lock = TRUE;
+#endif
}
if (res)
@@ -765,6 +935,9 @@
connection->io_path_acquired = TRUE;
}
+
+ _dbus_verbose ("%s end connection->io_path_acquired = %d res = %d\n",
+ _DBUS_FUNCTION_NAME, connection->io_path_acquired, res);
return res;
}
@@ -781,6 +954,9 @@
{
_dbus_assert (connection->io_path_acquired);
+ _dbus_verbose ("%s start connection->io_path_acquired = %d\n",
+ _DBUS_FUNCTION_NAME, connection->io_path_acquired);
+
connection->io_path_acquired = FALSE;
dbus_condvar_wake_one (connection->io_path_cond);
}
@@ -789,7 +965,7 @@
/**
* Queues incoming messages and sends outgoing messages for this
* connection, optionally blocking in the process. Each call to
- * _dbus_connection_do_iteration() will call select() or poll() one
+ * _dbus_connection_do_iteration_unlocked() will call select() or poll() one
* time and then read or write data if possible.
*
* The purpose of this function is to be able to flush outgoing
@@ -815,10 +991,14 @@
* @param timeout_milliseconds maximum blocking time, or -1 for no limit.
*/
void
-_dbus_connection_do_iteration (DBusConnection *connection,
- unsigned int flags,
- int timeout_milliseconds)
+_dbus_connection_do_iteration_unlocked (DBusConnection *connection,
+ unsigned int flags,
+ int timeout_milliseconds)
{
+ _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
+
+ HAVE_LOCK_CHECK (connection);
+
if (connection->n_outgoing == 0)
flags &= ~DBUS_ITERATION_DO_WRITING;
@@ -829,6 +1009,8 @@
flags, timeout_milliseconds);
_dbus_connection_release_io_path (connection);
}
+
+ _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
}
/**
@@ -949,11 +1131,15 @@
connection->client_serial = 1;
connection->disconnect_message_link = disconnect_link;
+
+ CONNECTION_LOCK (connection);
if (!_dbus_transport_set_connection (transport, connection))
goto error;
- _dbus_transport_ref (transport);
+ _dbus_transport_ref (transport);
+
+ CONNECTION_UNLOCK (connection);
return connection;
@@ -1006,9 +1192,11 @@
*/
DBusConnection *
_dbus_connection_ref_unlocked (DBusConnection *connection)
-{
+{
_dbus_assert (connection != NULL);
_dbus_assert (connection->generation == _dbus_current_generation);
+
+ HAVE_LOCK_CHECK (connection);
#ifdef DBUS_HAVE_ATOMIC_INT
_dbus_atomic_inc (&connection->refcount);
@@ -1031,7 +1219,9 @@
{
dbus_bool_t last_unref;
- _dbus_return_if_fail (connection != NULL);
+ HAVE_LOCK_CHECK (connection);
+
+ _dbus_assert (connection != NULL);
/* The connection lock is better than the global
* lock in the atomic increment fallback
@@ -1089,6 +1279,8 @@
DBusDispatchStatus status;
connection = data;
+
+ _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
CONNECTION_LOCK (connection);
_dbus_connection_acquire_io_path (connection, -1);
@@ -1096,10 +1288,16 @@
watch, condition);
_dbus_connection_release_io_path (connection);
+ HAVE_LOCK_CHECK (connection);
+
+ _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
+
status = _dbus_connection_get_dispatch_status_unlocked (connection);
/* this calls out to user code */
_dbus_connection_update_dispatch_status_and_unlock (connection, status);
+
+ _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
return retval;
}
@@ -1154,7 +1352,10 @@
dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
return NULL;
}
-
+
+#ifndef DBUS_DISABLE_CHECKS
+ _dbus_assert (!connection->have_connection_lock);
+#endif
return connection;
}
@@ -1357,7 +1558,8 @@
CONNECTION_LOCK (connection);
_dbus_transport_disconnect (connection->transport);
-
+
+ _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
status = _dbus_connection_get_dispatch_status_unlocked (connection);
/* this calls out to user code */
@@ -1367,6 +1569,7 @@
static dbus_bool_t
_dbus_connection_get_is_connected_unlocked (DBusConnection *connection)
{
+ HAVE_LOCK_CHECK (connection);
return _dbus_transport_get_is_connected (connection->transport);
}
@@ -1445,6 +1648,8 @@
{
DBusPreallocatedSend *preallocated;
+ HAVE_LOCK_CHECK (connection);
+
_dbus_assert (connection != NULL);
preallocated = dbus_new (DBusPreallocatedSend, 1);
@@ -1604,9 +1809,9 @@
/* Now we need to run an iteration to hopefully just write the messages
* out immediately, and otherwise get them queued up
*/
- _dbus_connection_do_iteration (connection,
- DBUS_ITERATION_DO_WRITING,
- -1);
+ _dbus_connection_do_iteration_unlocked (connection,
+ DBUS_ITERATION_DO_WRITING,
+ -1);
/* If stuff is still queued up, be sure we wake up the main loop */
if (connection->n_outgoing > 0)
@@ -1621,10 +1826,13 @@
{
DBusDispatchStatus status;
+ HAVE_LOCK_CHECK (connection);
+
_dbus_connection_send_preallocated_unlocked_no_update (connection,
preallocated,
message, client_serial);
+ _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
status = _dbus_connection_get_dispatch_status_unlocked (connection);
/* this calls out to user code */
@@ -1699,7 +1907,10 @@
preallocated = _dbus_connection_preallocate_send_unlocked (connection);
if (preallocated == NULL)
- return FALSE;
+ {
+ CONNECTION_UNLOCK (connection);
+ return FALSE;
+ }
_dbus_connection_send_preallocated_and_unlock (connection,
preallocated,
@@ -1762,6 +1973,7 @@
pending->timeout);
pending->timeout_added = FALSE;
+ _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
status = _dbus_connection_get_dispatch_status_unlocked (connection);
/* Unlocks, and calls out to user code */
@@ -1877,8 +2089,12 @@
if (pending_return)
*pending_return = pending;
else
- dbus_pending_call_unref (pending);
+ {
+ _dbus_connection_detach_pending_call_unlocked (connection, pending);
+ dbus_pending_call_unref (pending);
+ }
+ _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
status = _dbus_connection_get_dispatch_status_unlocked (connection);
/* this calls out to user code */
@@ -1898,6 +2114,8 @@
dbus_uint32_t client_serial)
{
DBusList *link;
+
+ HAVE_LOCK_CHECK (connection);
link = _dbus_list_get_first_link (&connection->incoming_messages);
@@ -1980,14 +2198,19 @@
* gets the message before we do?
*/
/* always block at least once as we know we don't have the reply yet */
- _dbus_connection_do_iteration (connection,
- DBUS_ITERATION_DO_READING |
- DBUS_ITERATION_BLOCK,
- timeout_milliseconds);
+ _dbus_connection_do_iteration_unlocked (connection,
+ DBUS_ITERATION_DO_READING |
+ DBUS_ITERATION_BLOCK,
+ timeout_milliseconds);
recheck_status:
+ _dbus_verbose ("%s top of recheck\n", _DBUS_FUNCTION_NAME);
+
+ HAVE_LOCK_CHECK (connection);
+
/* queue messages and get status */
+
status = _dbus_connection_get_dispatch_status_unlocked (connection);
if (status == DBUS_DISPATCH_DATA_REMAINS)
@@ -1996,7 +2219,8 @@
reply = check_for_reply_unlocked (connection, client_serial);
if (reply != NULL)
- {
+ {
+ _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME);
status = _dbus_connection_get_dispatch_status_unlocked (connection);
_dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n");
@@ -2011,7 +2235,10 @@
_dbus_get_current_time (&tv_sec, &tv_usec);
if (!_dbus_connection_get_is_connected_unlocked (connection))
- return NULL;
+ {
+ CONNECTION_UNLOCK (connection);
+ return NULL;
+ }
else if (tv_sec < start_tv_sec)
_dbus_verbose ("dbus_connection_send_with_reply_and_block(): clock set backward\n");
else if (connection->disconnect_message_link == NULL)
@@ -2042,10 +2269,10 @@
else
{
/* block again, we don't have the reply buffered yet. */
- _dbus_connection_do_iteration (connection,
- DBUS_ITERATION_DO_READING |
- DBUS_ITERATION_BLOCK,
- timeout_milliseconds);
+ _dbus_connection_do_iteration_unlocked (connection,
+ DBUS_ITERATION_DO_READING |
+ DBUS_ITERATION_BLOCK,
+ timeout_milliseconds);
}
goto recheck_status;
@@ -2144,16 +2371,25 @@
CONNECTION_LOCK (connection);
while (connection->n_outgoing > 0 &&
_dbus_connection_get_is_connected_unlocked (connection))
- _dbus_connection_do_iteration (connection,
- DBUS_ITERATION_DO_READING |
- DBUS_ITERATION_DO_WRITING |
- DBUS_ITERATION_BLOCK,
- -1);
+ {
+ _dbus_verbose ("doing iteration in %s\n", _DBUS_FUNCTION_NAME);
+ HAVE_LOCK_CHECK (connection);
+ _dbus_connection_do_iteration_unlocked (connection,
+ DBUS_ITERATION_DO_READING |
+ DBUS_ITERATION_DO_WRITING |
+ DBUS_ITERATION_BLOCK,
+ -1);
+ }
+ HAVE_LOCK_CHECK (connection);
+ _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
status = _dbus_connection_get_dispatch_status_unlocked (connection);
+ HAVE_LOCK_CHECK (connection);
/* Unlocks and calls out to user code */
_dbus_connection_update_dispatch_status_and_unlock (connection, status);
+
+ _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
}
/* Call with mutex held. Will drop it while waiting and re-acquire
@@ -2165,7 +2401,15 @@
_dbus_assert (connection->message_borrowed != NULL);
while (connection->message_borrowed != NULL)
- dbus_condvar_wait (connection->message_returned_cond, connection->mutex);
+ {
+#ifndef DBUS_DISABLE_CHECKS
+ connection->have_connection_lock = FALSE;
+#endif
+ dbus_condvar_wait (connection->message_returned_cond, connection->mutex);
+#ifndef DBUS_DISABLE_CHECKS
+ connection->have_connection_lock = TRUE;
+#endif
+ }
}
/**
@@ -2191,6 +2435,8 @@
_dbus_return_val_if_fail (connection != NULL, NULL);
/* can't borrow during dispatch */
_dbus_return_val_if_fail (!connection->dispatch_acquired, NULL);
+
+ _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
/* this is called for the side effect that it queues
* up any messages from the transport
@@ -2283,6 +2529,8 @@
static DBusList*
_dbus_connection_pop_message_link_unlocked (DBusConnection *connection)
{
+ HAVE_LOCK_CHECK (connection);
+
if (connection->message_borrowed != NULL)
_dbus_connection_wait_for_borrowed (connection);
@@ -2319,6 +2567,8 @@
_dbus_connection_pop_message_unlocked (DBusConnection *connection)
{
DBusList *link;
+
+ HAVE_LOCK_CHECK (connection);
link = _dbus_connection_pop_message_link_unlocked (connection);
@@ -2340,6 +2590,8 @@
_dbus_connection_putback_message_link_unlocked (DBusConnection *connection,
DBusList *message_link)
{
+ HAVE_LOCK_CHECK (connection);
+
_dbus_assert (message_link != NULL);
/* You can't borrow a message while a link is outstanding */
_dbus_assert (connection->message_borrowed == NULL);
@@ -2381,6 +2633,8 @@
DBusMessage *message;
DBusDispatchStatus status;
+ _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
+
/* this is called for the side effect that it queues
* up any messages from the transport
*/
@@ -2411,7 +2665,15 @@
_dbus_connection_acquire_dispatch (DBusConnection *connection)
{
if (connection->dispatch_acquired)
- dbus_condvar_wait (connection->dispatch_cond, connection->mutex);
+ {
+#ifndef DBUS_DISABLE_CHECKS
+ connection->have_connection_lock = FALSE;
+#endif
+ dbus_condvar_wait (connection->dispatch_cond, connection->mutex);
+#ifndef DBUS_DISABLE_CHECKS
+ connection->have_connection_lock = TRUE;
+#endif
+ }
_dbus_assert (!connection->dispatch_acquired);
connection->dispatch_acquired = TRUE;
@@ -2445,6 +2707,8 @@
static DBusDispatchStatus
_dbus_connection_get_dispatch_status_unlocked (DBusConnection *connection)
{
+ HAVE_LOCK_CHECK (connection);
+
if (connection->n_incoming > 0)
return DBUS_DISPATCH_DATA_REMAINS;
else if (!_dbus_transport_queue_messages (connection->transport))
@@ -2511,7 +2775,7 @@
DBusDispatchStatusFunction function;
void *data;
- /* We have the lock */
+ HAVE_LOCK_CHECK (connection);
_dbus_connection_ref_unlocked (connection);
@@ -2550,6 +2814,8 @@
DBusDispatchStatus status;
_dbus_return_val_if_fail (connection != NULL, DBUS_DISPATCH_COMPLETE);
+
+ _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
CONNECTION_LOCK (connection);
@@ -2759,7 +3025,8 @@
dbus_message_get_member (message) :
"no member",
dbus_message_get_signature (message));
-
+
+ HAVE_LOCK_CHECK (connection);
result = _dbus_object_tree_dispatch_and_unlock (connection->objects,
message);
@@ -2874,7 +3141,8 @@
}
_dbus_connection_release_dispatch (connection);
-
+
+ _dbus_verbose ("%s before final status update\n", _DBUS_FUNCTION_NAME);
status = _dbus_connection_get_dispatch_status_unlocked (connection);
/* unlocks and calls user code */
@@ -2952,25 +3220,43 @@
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 ("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);
- /* FIXME this can call back into user code, and we need to drop the
- * connection lock when it does.
+ /* 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.
*/
- retval = _dbus_watch_list_set_functions (connection->watches,
+ watches = connection->watches;
+ connection->watches = NULL;
+ CONNECTION_UNLOCK (connection);
+
+ retval = _dbus_watch_list_set_functions (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;
}
@@ -3016,17 +3302,34 @@
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 ("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 (connection->timeouts,
+ retval = _dbus_timeout_list_set_functions (timeouts,
add_function, remove_function,
toggled_function,
data, free_data_function);
+ CONNECTION_LOCK (connection);
+ connection->timeouts = timeouts;
CONNECTION_UNLOCK (connection);
/* drop our paranoid refcount */
Index: dbus-object-tree.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-object-tree.c,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -d -r1.13 -r1.14
--- dbus-object-tree.c 10 Feb 2005 23:47:55 -0000 1.13
+++ dbus-object-tree.c 13 Feb 2005 17:16:25 -0000 1.14
@@ -1,7 +1,7 @@
/* -*- mode: C; c-file-style: "gnu" -*- */
/* dbus-object-tree.c DBusObjectTree (internals of DBusConnection)
*
- * Copyright (C) 2003 Red Hat Inc.
+ * Copyright (C) 2003, 2005 Red Hat Inc.
*
* Licensed under the Academic Free License version 2.1
*
@@ -503,6 +503,7 @@
#endif
{
_dbus_connection_ref_unlocked (connection);
+ _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
_dbus_connection_unlock (connection);
}
@@ -635,7 +636,10 @@
#ifdef DBUS_BUILD_TESTS
if (tree->connection)
#endif
- _dbus_connection_unlock (tree->connection);
+ {
+ _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
+ _dbus_connection_unlock (tree->connection);
+ }
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
@@ -647,7 +651,10 @@
#ifdef DBUS_BUILD_TESTS
if (tree->connection)
#endif
- _dbus_connection_unlock (tree->connection);
+ {
+ _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
+ _dbus_connection_unlock (tree->connection);
+ }
return DBUS_HANDLER_RESULT_NEED_MEMORY;
}
@@ -690,6 +697,8 @@
if (tree->connection)
#endif
{
+ already_unlocked = TRUE;
+
if (!_dbus_connection_send_and_unlock (tree->connection, reply, NULL))
goto out;
}
@@ -702,7 +711,10 @@
#endif
{
if (!already_unlocked)
- _dbus_connection_unlock (tree->connection);
+ {
+ _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
+ _dbus_connection_unlock (tree->connection);
+ }
}
_dbus_string_free (&xml);
@@ -747,7 +759,10 @@
#ifdef DBUS_BUILD_TESTS
if (tree->connection)
#endif
- _dbus_connection_unlock (tree->connection);
+ {
+ _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_unlock (tree->connection);
+ }
_dbus_verbose ("No memory to get decomposed path\n");
@@ -759,7 +774,10 @@
#ifdef DBUS_BUILD_TESTS
if (tree->connection)
#endif
- _dbus_connection_unlock (tree->connection);
+ {
+ _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_unlock (tree->connection);
+ }
_dbus_verbose ("No path field in message\n");
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -822,7 +840,10 @@
#ifdef DBUS_BUILD_TESTS
if (tree->connection)
#endif
- _dbus_connection_unlock (tree->connection);
+ {
+ _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_unlock (tree->connection);
+ }
/* FIXME you could unregister the subtree in another thread
* before we invoke the callback, and I can't figure out a
@@ -859,7 +880,10 @@
#ifdef DBUS_BUILD_TESTS
if (tree->connection)
#endif
- _dbus_connection_unlock (tree->connection);
+ {
+ _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_unlock (tree->connection);
+ }
}
while (list != NULL)
@@ -993,7 +1017,10 @@
#ifdef DBUS_BUILD_TESTS
if (tree->connection)
#endif
- _dbus_connection_unlock (tree->connection);
+ {
+ _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_unlock (tree->connection);
+ }
return result;
}
Index: dbus-server-protected.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server-protected.h,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -d -r1.14 -r1.15
--- dbus-server-protected.h 9 Sep 2004 10:20:17 -0000 1.14
+++ dbus-server-protected.h 13 Feb 2005 17:16:25 -0000 1.15
@@ -23,6 +23,7 @@
#ifndef DBUS_SERVER_PROTECTED_H
#define DBUS_SERVER_PROTECTED_H
+#include <config.h>
#include <dbus/dbus-internals.h>
#include <dbus/dbus-server.h>
#include <dbus/dbus-timeout.h>
@@ -51,8 +52,9 @@
*/
struct DBusServer
{
- int refcount; /**< Reference count. */
+ DBusAtomic refcount; /**< Reference count. */
const DBusServerVTable *vtable; /**< Virtual methods for this instance. */
+ DBusMutex *mutex; /**< Lock on the server object */
DBusWatchList *watches; /**< Our watches */
DBusTimeoutList *timeouts; /**< Our timeouts */
@@ -74,6 +76,10 @@
char **auth_mechanisms; /**< Array of allowed authentication mechanisms */
unsigned int disconnected : 1; /**< TRUE if we are disconnected. */
+
+#ifndef DBUS_DISABLE_CHECKS
+ unsigned int have_server_lock : 1; /**< Does someone have the server mutex locked */
+#endif
};
dbus_bool_t _dbus_server_init_base (DBusServer *server,
@@ -95,7 +101,38 @@
DBusTimeout *timeout,
dbus_bool_t enabled);
+void _dbus_server_ref_unlocked (DBusServer *server);
+#ifdef DBUS_DISABLE_CHECKS
+#define TOOK_LOCK_CHECK(server)
+#define RELEASING_LOCK_CHECK(server)
+#define HAVE_LOCK_CHECK(server)
+#else
+#define TOOK_LOCK_CHECK(server) do { \
+ _dbus_assert (!(server)->have_server_lock); \
+ (server)->have_server_lock = TRUE; \
+ } while (0)
+#define RELEASING_LOCK_CHECK(server) do { \
+ _dbus_assert ((server)->have_server_lock); \
+ (server)->have_server_lock = FALSE; \
+ } while (0)
+#define HAVE_LOCK_CHECK(server) _dbus_assert ((server)->have_server_lock)
+/* A "DO_NOT_HAVE_LOCK_CHECK" is impossible since we need the lock to check the flag */
+#endif
+
+#define TRACE_LOCKS 0
+
+#define SERVER_LOCK(server) do { \
+ if (TRACE_LOCKS) { _dbus_verbose (" LOCK: %s\n", _DBUS_FUNCTION_NAME); } \
+ dbus_mutex_lock ((server)->mutex); \
+ TOOK_LOCK_CHECK (server); \
+ } while (0)
+
+#define SERVER_UNLOCK(server) do { \
+ if (TRACE_LOCKS) { _dbus_verbose (" UNLOCK: %s\n", _DBUS_FUNCTION_NAME); } \
+ RELEASING_LOCK_CHECK (server); \
+ dbus_mutex_unlock ((server)->mutex); \
+ } while (0)
DBUS_END_DECLS
Index: dbus-server-unix.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server-unix.c,v
retrieving revision 1.25
retrieving revision 1.26
diff -u -d -r1.25 -r1.26
--- dbus-server-unix.c 11 Feb 2005 03:37:03 -0000 1.25
+++ dbus-server-unix.c 13 Feb 2005 17:16:25 -0000 1.26
@@ -72,21 +72,29 @@
*/
/* Return value is just for memory, not other failures. */
static dbus_bool_t
-handle_new_client_fd (DBusServer *server,
- int client_fd)
+handle_new_client_fd_and_unlock (DBusServer *server,
+ int client_fd)
{
DBusConnection *connection;
DBusTransport *transport;
+ DBusNewConnectionFunction new_connection_function;
+ void *new_connection_data;
_dbus_verbose ("Creating new client connection with fd %d\n", client_fd);
-
+
+ HAVE_LOCK_CHECK (server);
+
if (!_dbus_set_fd_nonblocking (client_fd, NULL))
- return TRUE;
+ {
+ SERVER_UNLOCK (server);
+ return TRUE;
+ }
transport = _dbus_transport_new_for_fd (client_fd, TRUE, NULL);
if (transport == NULL)
{
close (client_fd);
+ SERVER_UNLOCK (server);
return FALSE;
}
@@ -94,6 +102,7 @@
(const char **) server->auth_mechanisms))
{
_dbus_transport_unref (transport);
+ SERVER_UNLOCK (server);
return FALSE;
}
@@ -103,19 +112,27 @@
connection = _dbus_connection_new_for_transport (transport);
_dbus_transport_unref (transport);
+ transport = NULL; /* now under the connection lock */
if (connection == NULL)
- return FALSE;
+ {
+ SERVER_UNLOCK (server);
+ return FALSE;
+ }
- /* See if someone wants to handle this new connection,
- * self-referencing for paranoia
+ /* See if someone wants to handle this new connection, self-referencing
+ * for paranoia.
*/
- if (server->new_connection_function)
+ new_connection_function = server->new_connection_function;
+ new_connection_data = server->new_connection_data;
+
+ _dbus_server_ref_unlocked (server);
+ SERVER_UNLOCK (server);
+
+ if (new_connection_function)
{
- dbus_server_ref (server);
-
- (* server->new_connection_function) (server, connection,
- server->new_connection_data);
+ (* new_connection_function) (server, connection,
+ new_connection_data);
dbus_server_unref (server);
}
@@ -133,6 +150,8 @@
DBusServer *server = data;
DBusServerUnix *unix_server = data;
+ SERVER_LOCK (server);
+
_dbus_assert (watch == unix_server->watch);
_dbus_verbose ("Handling client connection, flags 0x%x\n", flags);
@@ -155,12 +174,14 @@
else
_dbus_verbose ("Failed to accept a client connection: %s\n",
_dbus_strerror (errno));
+
+ SERVER_UNLOCK (server);
}
else
{
_dbus_fd_set_close_on_exec (client_fd);
- if (!handle_new_client_fd (server, client_fd))
+ if (!handle_new_client_fd_and_unlock (server, client_fd))
_dbus_verbose ("Rejected client connection due to lack of memory\n");
}
}
@@ -246,6 +267,10 @@
return NULL;
}
+#ifndef DBUS_DISABLE_CHECKS
+ unix_server->base.have_server_lock = TRUE;
+#endif
+
if (!_dbus_server_add_watch (&unix_server->base,
watch))
{
@@ -254,6 +279,10 @@
dbus_free (unix_server);
return NULL;
}
+
+#ifndef DBUS_DISABLE_CHECKS
+ unix_server->base.have_server_lock = FALSE;
+#endif
unix_server->fd = fd;
unix_server->watch = watch;
Index: dbus-server.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server.c,v
retrieving revision 1.36
retrieving revision 1.37
diff -u -d -r1.36 -r1.37
--- dbus-server.c 10 Aug 2004 03:07:00 -0000 1.36
+++ dbus-server.c 13 Feb 2005 17:16:25 -0000 1.37
@@ -67,7 +67,7 @@
const DBusString *address)
{
server->vtable = vtable;
- server->refcount = 1;
+ server->refcount.value = 1;
server->address = NULL;
server->watches = NULL;
@@ -75,6 +75,10 @@
if (!_dbus_string_copy_data (address, &server->address))
goto failed;
+
+ server->mutex = dbus_mutex_new ();
+ if (server->mutex == NULL)
+ goto failed;
server->watches = _dbus_watch_list_new ();
if (server->watches == NULL)
@@ -91,6 +95,11 @@
return TRUE;
failed:
+ if (server->mutex)
+ {
+ dbus_mutex_free (server->mutex);
+ server->mutex = NULL;
+ }
if (server->watches)
{
_dbus_watch_list_free (server->watches);
@@ -118,7 +127,7 @@
*/
void
_dbus_server_finalize_base (DBusServer *server)
-{
+{
/* calls out to application code... */
_dbus_data_slot_list_free (&server->slot_list);
@@ -130,6 +139,8 @@
_dbus_watch_list_free (server->watches);
_dbus_timeout_list_free (server->timeouts);
+ dbus_mutex_free (server->mutex);
+
dbus_free (server->address);
dbus_free_string_array (server->auth_mechanisms);
@@ -146,6 +157,7 @@
_dbus_server_add_watch (DBusServer *server,
DBusWatch *watch)
{
+ HAVE_LOCK_CHECK (server);
return _dbus_watch_list_add_watch (server->watches, watch);
}
@@ -159,6 +171,7 @@
_dbus_server_remove_watch (DBusServer *server,
DBusWatch *watch)
{
+ HAVE_LOCK_CHECK (server);
_dbus_watch_list_remove_watch (server->watches, watch);
}
@@ -176,6 +189,8 @@
DBusWatch *watch,
dbus_bool_t enabled)
{
+ HAVE_LOCK_CHECK (server);
+
if (server->watches) /* null during finalize */
_dbus_watch_list_toggle_watch (server->watches,
watch, enabled);
@@ -194,6 +209,8 @@
_dbus_server_add_timeout (DBusServer *server,
DBusTimeout *timeout)
{
+ HAVE_LOCK_CHECK (server);
+
return _dbus_timeout_list_add_timeout (server->timeouts, timeout);
}
@@ -207,6 +224,8 @@
_dbus_server_remove_timeout (DBusServer *server,
DBusTimeout *timeout)
{
+ HAVE_LOCK_CHECK (server);
+
_dbus_timeout_list_remove_timeout (server->timeouts, timeout);
}
@@ -224,6 +243,8 @@
DBusTimeout *timeout,
dbus_bool_t enabled)
{
+ HAVE_LOCK_CHECK (server);
+
if (server->timeouts) /* null during finalize */
_dbus_timeout_list_toggle_timeout (server->timeouts,
timeout, enabled);
@@ -457,8 +478,16 @@
dbus_server_ref (DBusServer *server)
{
_dbus_return_val_if_fail (server != NULL, NULL);
-
- server->refcount += 1;
+
+#ifdef DBUS_HAVE_ATOMIC_INT
+ _dbus_atomic_inc (&server->refcount);
+#else
+ SERVER_LOCK (server);
+ _dbus_assert (server->refcount.value > 0);
+
+ server->refcount.value += 1;
+ SERVER_UNLOCK (server);
+#endif
return server;
}
@@ -474,12 +503,24 @@
void
dbus_server_unref (DBusServer *server)
{
+ dbus_bool_t last_unref;
+
_dbus_return_if_fail (server != NULL);
- _dbus_assert (server->refcount > 0);
+#ifdef DBUS_HAVE_ATOMIC_INT
+ last_unref = (_dbus_atomic_dec (&server->refcount) == 1);
+#else
+ SERVER_LOCK (server);
+
+ _dbus_assert (server->refcount.value > 0);
- server->refcount -= 1;
- if (server->refcount == 0)
+ server->refcount.value -= 1;
+ last_unref = (server->refcount.value == 0);
+
+ SERVER_UNLOCK (server);
+#endif
+
+ if (last_unref)
{
_dbus_assert (server->vtable->finalize != NULL);
@@ -488,6 +529,25 @@
}
/**
+ * Like dbus_server_ref() but does not acquire the lock (must already be held)
+ *
+ * @param server the server.
+ */
+void
+_dbus_server_ref_unlocked (DBusServer *server)
+{
+ HAVE_LOCK_CHECK (server);
+
+#ifdef DBUS_HAVE_ATOMIC_INT
+ _dbus_atomic_inc (&server->refcount);
+#else
+ _dbus_assert (server->refcount.value > 0);
+
+ server->refcount.value += 1;
+#endif
+}
+
+/**
* Releases the server's address and stops listening for
* new clients. If called more than once, only the first
* call has an effect. Does not modify the server's
@@ -499,6 +559,8 @@
dbus_server_disconnect (DBusServer *server)
{
_dbus_return_if_fail (server != NULL);
+
+ SERVER_LOCK (server);
_dbus_assert (server->vtable->disconnect != NULL);
@@ -507,6 +569,8 @@
(* server->vtable->disconnect) (server);
server->disconnected = TRUE;
+
+ SERVER_UNLOCK (server);
}
/**
@@ -517,9 +581,15 @@
dbus_bool_t
dbus_server_get_is_connected (DBusServer *server)
{
- _dbus_return_val_if_fail (server != NULL, FALSE);
+ dbus_bool_t retval;
- return !server->disconnected;
+ _dbus_return_val_if_fail (server != NULL, FALSE);
+
+ SERVER_LOCK (server);
+ retval = !server->disconnected;
+ SERVER_UNLOCK (server);
+
+ return retval;
}
/**
@@ -532,9 +602,15 @@
char*
dbus_server_get_address (DBusServer *server)
{
- _dbus_return_val_if_fail (server != NULL, NULL);
+ char *retval;
- return _dbus_strdup (server->address);
+ _dbus_return_val_if_fail (server != NULL, NULL);
+
+ SERVER_LOCK (server);
+ retval = _dbus_strdup (server->address);
+ SERVER_UNLOCK (server);
+
+ return retval;
}
/**
@@ -555,14 +631,22 @@
void *data,
DBusFreeFunction free_data_function)
{
- _dbus_return_if_fail (server != NULL);
+ DBusFreeFunction old_free_function;
+ void *old_data;
- if (server->new_connection_free_data_function != NULL)
- (* server->new_connection_free_data_function) (server->new_connection_data);
+ _dbus_return_if_fail (server != NULL);
+
+ SERVER_LOCK (server);
+ old_free_function = server->new_connection_free_data_function;
+ old_data = server->new_connection_data;
server->new_connection_function = function;
server->new_connection_data = data;
server->new_connection_free_data_function = free_data_function;
+ SERVER_UNLOCK (server);
+
+ if (old_free_function != NULL)
+ (* old_free_function) (old_data);
}
/**
@@ -589,14 +673,34 @@
void *data,
DBusFreeFunction free_data_function)
{
+ dbus_bool_t result;
+ DBusWatchList *watches;
+
_dbus_return_val_if_fail (server != NULL, FALSE);
+
+ SERVER_LOCK (server);
+ watches = server->watches;
+ server->watches = NULL;
+ if (watches)
+ {
+ SERVER_UNLOCK (server);
+ result = _dbus_watch_list_set_functions (watches,
+ add_function,
+ remove_function,
+ toggled_function,
+ data,
+ free_data_function);
+ SERVER_LOCK (server);
+ }
+ else
+ {
+ _dbus_warn ("Re-entrant call to %s\n", _DBUS_FUNCTION_NAME);
+ result = FALSE;
+ }
+ server->watches = watches;
+ SERVER_UNLOCK (server);
- return _dbus_watch_list_set_functions (server->watches,
- add_function,
- remove_function,
- toggled_function,
- data,
- free_data_function);
+ return result;
}
/**
@@ -622,12 +726,34 @@
void *data,
DBusFreeFunction free_data_function)
{
+ dbus_bool_t result;
+ DBusTimeoutList *timeouts;
+
_dbus_return_val_if_fail (server != NULL, FALSE);
+
+ SERVER_LOCK (server);
+ timeouts = server->timeouts;
+ server->timeouts = NULL;
+ if (timeouts)
+ {
+ SERVER_UNLOCK (server);
+ result = _dbus_timeout_list_set_functions (timeouts,
+ add_function,
+ remove_function,
+ toggled_function,
+ data,
+ free_data_function);
+ SERVER_LOCK (server);
+ }
+ else
+ {
+ _dbus_warn ("Re-entrant call to %s\n", _DBUS_FUNCTION_NAME);
+ result = FALSE;
+ }
+ server->timeouts = timeouts;
+ SERVER_UNLOCK (server);
- return _dbus_timeout_list_set_functions (server->timeouts,
- add_function, remove_function,
- toggled_function,
- data, free_data_function);
+ return result;
}
/**
@@ -647,6 +773,8 @@
char **copy;
_dbus_return_val_if_fail (server != NULL, FALSE);
+
+ SERVER_LOCK (server);
if (mechanisms != NULL)
{
@@ -660,6 +788,8 @@
dbus_free_string_array (server->auth_mechanisms);
server->auth_mechanisms = copy;
+ SERVER_UNLOCK (server);
+
return TRUE;
}
@@ -732,19 +862,16 @@
dbus_bool_t retval;
_dbus_return_val_if_fail (server != NULL, FALSE);
-
-#if 0
- dbus_mutex_lock (server->mutex);
-#endif
+
+ SERVER_LOCK (server);
retval = _dbus_data_slot_list_set (&slot_allocator,
&server->slot_list,
slot, data, free_data_func,
&old_free_func, &old_data);
-#if 0
- dbus_mutex_unlock (server->mutex);
-#endif
+
+ SERVER_UNLOCK (server);
if (retval)
{
@@ -772,17 +899,13 @@
_dbus_return_val_if_fail (server != NULL, NULL);
-#if 0
- dbus_mutex_lock (server->mutex);
-#endif
+ SERVER_LOCK (server);
res = _dbus_data_slot_list_get (&slot_allocator,
&server->slot_list,
slot);
-#if 0
- dbus_mutex_unlock (server->mutex);
-#endif
+ SERVER_UNLOCK (server);
return res;
}
Index: dbus-transport-unix.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-transport-unix.c,v
retrieving revision 1.44
retrieving revision 1.45
diff -u -d -r1.44 -r1.45
--- dbus-transport-unix.c 26 Nov 2004 02:29:00 -0000 1.44
+++ dbus-transport-unix.c 13 Feb 2005 17:16:25 -0000 1.45
@@ -70,6 +70,8 @@
free_watches (DBusTransport *transport)
{
DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport;
+
+ _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
if (unix_transport->read_watch)
{
@@ -90,12 +92,16 @@
_dbus_watch_unref (unix_transport->write_watch);
unix_transport->write_watch = NULL;
}
+
+ _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
}
static void
unix_finalize (DBusTransport *transport)
{
DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport;
+
+ _dbus_verbose ("%s\n", _DBUS_FUNCTION_NAME);
free_watches (transport);
@@ -871,6 +877,8 @@
unix_disconnect (DBusTransport *transport)
{
DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport;
+
+ _dbus_verbose ("%s\n", _DBUS_FUNCTION_NAME);
free_watches (transport);
@@ -1004,7 +1012,10 @@
* by the io_path_cond condvar, so we won't reenter this.
*/
if (flags & DBUS_ITERATION_BLOCK)
- _dbus_connection_unlock (transport->connection);
+ {
+ _dbus_verbose ("unlock %s pre poll\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_unlock (transport->connection);
+ }
again:
poll_res = _dbus_poll (&poll_fd, 1, poll_timeout);
@@ -1013,7 +1024,10 @@
goto again;
if (flags & DBUS_ITERATION_BLOCK)
- _dbus_connection_lock (transport->connection);
+ {
+ _dbus_verbose ("lock %s post poll\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_lock (transport->connection);
+ }
if (poll_res >= 0)
{
Index: dbus-transport.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-transport.c,v
retrieving revision 1.43
retrieving revision 1.44
diff -u -d -r1.43 -r1.44
--- dbus-transport.c 28 Jan 2005 03:06:56 -0000 1.43
+++ dbus-transport.c 13 Feb 2005 17:16:25 -0000 1.44
@@ -388,10 +388,12 @@
{
_dbus_assert (transport != NULL);
_dbus_assert (transport->refcount > 0);
-
+
transport->refcount -= 1;
if (transport->refcount == 0)
{
+ _dbus_verbose ("%s: finalizing\n", _DBUS_FUNCTION_NAME);
+
_dbus_assert (transport->vtable->finalize != NULL);
(* transport->vtable->finalize) (transport);
@@ -409,14 +411,18 @@
void
_dbus_transport_disconnect (DBusTransport *transport)
{
+ _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
+
_dbus_assert (transport->vtable->disconnect != NULL);
-
+
if (transport->disconnected)
return;
(* transport->vtable->disconnect) (transport);
transport->disconnected = TRUE;
+
+ _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
}
/**
@@ -437,7 +443,8 @@
* Returns #TRUE if we have been authenticated. Will return #TRUE
* even if the transport is disconnected.
*
- * @todo needs to drop connection->mutex when calling the unix_user_function
+ * @todo we drop connection->mutex when calling the unix_user_function,
+ * which may not be safe really.
*
* @param transport the transport
* @returns whether we're authenticated
@@ -453,6 +460,9 @@
if (transport->disconnected)
return FALSE;
+
+ /* paranoia ref since we call user callbacks sometimes */
+ _dbus_connection_ref_unlocked (transport->connection);
maybe_authenticated =
(!(transport->send_credentials_pending ||
@@ -486,21 +496,40 @@
if (transport->unix_user_function != NULL)
{
- /* FIXME we hold the connection lock here and should drop it */
- if (!(* transport->unix_user_function) (transport->connection,
- auth_identity.uid,
- transport->unix_user_data))
+ dbus_bool_t allow;
+ DBusConnection *connection;
+ DBusAllowUnixUserFunction unix_user_function;
+ void *unix_user_data;
+
+ /* Dropping the lock here probably isn't that safe. */
+
+ connection = transport->connection;
+ unix_user_function = transport->unix_user_function;
+ unix_user_data = transport->unix_user_data;
+
+ _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_unlock (connection);
+
+ allow = (* unix_user_function) (connection,
+ auth_identity.uid,
+ unix_user_data);
+
+ _dbus_verbose ("lock %s post unix user function\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_lock (connection);
+
+ if (allow)
+ {
+ _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", auth_identity.uid);
+ }
+ else
{
_dbus_verbose ("Client UID "DBUS_UID_FORMAT
" was rejected, disconnecting\n",
auth_identity.uid);
_dbus_transport_disconnect (transport);
+ _dbus_connection_unref_unlocked (connection);
return FALSE;
}
- else
- {
- _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", auth_identity.uid);
- }
}
else
{
@@ -515,6 +544,7 @@
" but our UID is "DBUS_UID_FORMAT", disconnecting\n",
auth_identity.uid, our_identity.uid);
_dbus_transport_disconnect (transport);
+ _dbus_connection_unref_unlocked (transport->connection);
return FALSE;
}
else
@@ -527,8 +557,9 @@
}
transport->authenticated = maybe_authenticated;
-
- return transport->authenticated;
+
+ _dbus_connection_unref_unlocked (transport->connection);
+ return maybe_authenticated;
}
}
@@ -670,6 +701,8 @@
(* transport->vtable->do_iteration) (transport, flags,
timeout_milliseconds);
_dbus_transport_unref (transport);
+
+ _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
}
static dbus_bool_t
More information about the dbus-commit
mailing list