From dc80f329a375aaba00e98d982f510fd2ae1a358a Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Tue, 26 May 2020 01:10:02 +0200 Subject: [PATCH 1/4] Use better name for bus related function named transaction_free () --- bus/connection.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bus/connection.c b/bus/connection.c index add1c8b8..ff79d705 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -2527,7 +2527,7 @@ bus_transaction_send (BusTransaction *transaction, } static void -transaction_free (BusTransaction *transaction) +bus_transaction_free (BusTransaction *transaction) { _dbus_assert (transaction->connections == NULL); @@ -2577,7 +2577,7 @@ bus_transaction_cancel_and_free (BusTransaction *transaction) _dbus_list_foreach (&transaction->cancel_hooks, cancel_hook_cancel, NULL); - transaction_free (transaction); + bus_transaction_free (transaction); } static void @@ -2631,7 +2631,7 @@ bus_transaction_execute_and_free (BusTransaction *transaction) while ((connection = _dbus_list_pop_first (&transaction->connections))) connection_execute_transaction (connection, transaction); - transaction_free (transaction); + bus_transaction_free (transaction); } static void From 404de07ac9e524380e7784da349ea60cd15aaf2f Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 26 May 2020 01:27:55 +0200 Subject: [PATCH 2/4] Add api doc section BusInternals and BusTransaction Since the functions associated with bus transactions are distributed over the file, it is necessary to define several @ingroup blocks. --- bus/bus.c | 4 +++ bus/connection.c | 76 ++++++++++++++++++++++++++++++++++++++++++ dbus/dbus-connection.c | 2 +- 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/bus/bus.c b/bus/bus.c index db20bbbc..ec2ee072 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -50,6 +50,10 @@ #include #endif +/** @defgroup BusInternals + * @brief Internals of the dbus server implementation + */ + struct BusContext { int refcount; diff --git a/bus/connection.c b/bus/connection.c index ff79d705..be5aa8a0 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -2248,6 +2248,59 @@ free_cancel_hooks (BusTransaction *transaction) _dbus_list_clear (&transaction->cancel_hooks); } +/** + * @defgroup BusTransaction + * @ingroup BusInternals + * @brief Symbolizes a unit of work performed within the dbus server + * and treated in a coherent and reliable way independent of other transactions + * + * The purpose of BusTransaction is to avoid this situation: + * + * - We dispatch a message + * - Dispatching that message results in new messages + * -# it could be a message from a sender, through the dbus-daemon, + * to one or more destinations (broadcast signals can go to several + * destinations, and legacy eavesdropping and the more recent + * monitoring can also result in unicast messages having to go + * to more than one destination) + * -# or it could be a message from a sender to the dbus-daemon itself + * (the "bus driver", org.freedesktop.DBus) that sends other + * messages as a side-effect (for example a successful RequestName() + * results in NameOwnerChanged and NameAcquired signals, in addition + * to the reply to RequestName) + * - We send one of the resulting messages (let's say M1), and it succeeds + * - We try to send another of the resulting messages (let's say M2), but + * we run out of memory and cannot send it + * - Now what? + * -# We can't take back M1, because we already sent it + * -# We can't send M2, because we don't have enough memory + * -# Sending M1 but not M2 would leave recipients with an inconsistent + * view of the state of the world + * + * So, instead, we use transactions. They're conceptually similar to + * database transactions: you start with an empty transaction, you add + * actions (in our case messages), and eventually you either execute the + * transaction or cancel it (similar to committing or rolling back a + * database transaction). + * To make sure we have enough memory, every time we add a message, we + * preallocate enough memory to "send" that message. When we execute + * (commit) the transaction, we use that memory to "send" the messages. + * I say "send" here because dbus-daemon is not actually aware of the + * OS-level reality of what is happening to the messages: it's just + * using the libdbus abstractions. Even after we execute a BusTransaction, + * we still aren't actually physically sending the message! All we are + * doing is moving the message from the linked list in the BusTransaction + * to the linked list in the DBusConnection. We cannot actually start + * sending the message until there is enough space for at least its first + * byte in the AF_UNIX or TCP socket's buffers, which is determined by + * poll(), select() or epoll_wait() - and we cannot finish sending the + * message until there is enough space for its last byte, which might + * take a lot of iterations for a large message. + * This gap between dbus_connection_send() and OS-level send() is described + * in the @ref use_dbus_connection_send "DBusConnection introduction". + * @{ + */ + BusTransaction* bus_transaction_new (BusContext *context) { @@ -2536,6 +2589,8 @@ bus_transaction_free (BusTransaction *transaction) dbus_free (transaction); } +/** @} */ /* end of BusTransaction */ + static void connection_cancel_transaction (DBusConnection *connection, BusTransaction *transaction) @@ -2564,6 +2619,11 @@ connection_cancel_transaction (DBusConnection *connection, } } +/** + * @addtogroup BusTransaction + * @{ + */ + void bus_transaction_cancel_and_free (BusTransaction *transaction) { @@ -2580,6 +2640,8 @@ bus_transaction_cancel_and_free (BusTransaction *transaction) bus_transaction_free (transaction); } +/** @} */ /* end of BusTransaction */ + static void connection_execute_transaction (DBusConnection *connection, BusTransaction *transaction) @@ -2618,6 +2680,11 @@ connection_execute_transaction (DBusConnection *connection, } } +/** + * @addtogroup BusTransaction + * @{ + */ + void bus_transaction_execute_and_free (BusTransaction *transaction) { @@ -2634,6 +2701,8 @@ bus_transaction_execute_and_free (BusTransaction *transaction) bus_transaction_free (transaction); } +/** @} */ /* end of BusTransaction */ + static void bus_connection_remove_transactions (DBusConnection *connection) { @@ -2654,6 +2723,11 @@ bus_connection_remove_transactions (DBusConnection *connection) } } +/** + * @addtogroup BusTransaction + * @{ + */ + /** * Converts the DBusError to a message reply */ @@ -2719,6 +2793,8 @@ bus_transaction_add_cancel_hook (BusTransaction *transaction, return TRUE; } +/** @} */ /* end of BusTransaction */ + int bus_connections_get_n_active (BusConnections *connections) { diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 159dbe1b..47f21ae3 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -152,7 +152,7 @@ * dbus_connection_add_filter(), * dbus_connection_register_object_path() for more on handlers). * - * When you use dbus_connection_send() or one of its variants to send + * @anchor use_dbus_connection_send When you use dbus_connection_send() or one of its variants to send * a message, the message is added to the outgoing queue. It's * actually written to the network later; either in * dbus_watch_handle() invoked by your main loop, or in From 23cb69d641efb3f7dd733bd5946ef97b03c24e12 Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Tue, 26 May 2020 01:28:36 +0200 Subject: [PATCH 3/4] Add api doc for bus_transaction_send() and bus_transaction_send_from_driver() --- bus/connection.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/bus/connection.c b/bus/connection.c index be5aa8a0..a5b0d405 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -2405,6 +2405,27 @@ out: return ret; } +/** + * @brief Adds a message from the bus driver to be sent when the transaction is executed + * + * The function prepends a message from a bus driver to a transcation + * to be sent in a later main-loop iteration and does not expect + * to get a reply. + * + * If security policy doesn't allow the message, we would silently + * eat it; the driver doesn't care about getting a reply. However, + * if we're actively capturing messages, it's nice to log that we + * tried to send it and did not allow ourselves to do so. + * + * + * @param transaction transaction to prepend to message + * @param connection connection to send the message to + * @param message the message to send + * @return TRUE message is added to transaction or ignored + * @return FALSE message is not added to the transaction + * + * See bus_transaction_send() and @ref BusTransaction for further details. + */ dbus_bool_t bus_transaction_send_from_driver (BusTransaction *transaction, DBusConnection *connection, @@ -2470,6 +2491,25 @@ bus_transaction_send_from_driver (BusTransaction *transaction, return bus_transaction_send (transaction, NULL, connection, message); } +/** + * @brief Adds a message to be sent when the transaction is executed + * + * Add a message from @sender to the transaction. If there is enough + * memory to process the entire transaction, the message will be sent to + * @destination when the transaction is executed. If not, it will be dropped + * when the transaction is cancelled. + * + * If the destination is disconnected, the message is silently ignored. + * This is treated as a success, and #TRUE is returned. + * + * @param transaction transaction to prepend to message + * @param sender sender for the message + * @param destination where to send the message to + * @param message the message to send + * @return #TRUE on success, or #FALSE if not enough memory. + * + * See @ref BusTransaction for further details. + */ dbus_bool_t bus_transaction_send (BusTransaction *transaction, DBusConnection *sender, From 55c022ff5013763f282107635f4d1c6f1f0f50c3 Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Tue, 26 May 2020 01:28:54 +0200 Subject: [PATCH 4/4] Add api documention to bus_transaction_new() and bus_transaction_free() --- bus/connection.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/bus/connection.c b/bus/connection.c index a5b0d405..fc9f76fb 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -2301,6 +2301,12 @@ free_cancel_hooks (BusTransaction *transaction) * @{ */ +/** + * Creates a new instance of type @ref BusTransaction + * + * @param context bus context to add to the transaction + * @return allocated memory, or #NULL if the allocation fails. + */ BusTransaction* bus_transaction_new (BusContext *context) { @@ -2315,6 +2321,12 @@ bus_transaction_new (BusContext *context) return transaction; } +/** + * Return bus context from the given @ref BusTransaction instance + * + * @param transaction transaction to get the context from + * @return bus context + */ BusContext* bus_transaction_get_context (BusTransaction *transaction) { @@ -2619,6 +2631,11 @@ bus_transaction_send (BusTransaction *transaction, return TRUE; } +/** + * Destroy @ref BusTransaction instance + * + * @param transaction instance to destroy + */ static void bus_transaction_free (BusTransaction *transaction) {