Commit 6be547d3 authored by Havoc Pennington's avatar Havoc Pennington

2003-04-10 Havoc Pennington <hp@redhat.com>

	* dbus/dbus-connection.c (dbus_connection_flush): don't spin on
	the connection if it's disconnected

	* bus/activation.c (bus_activation_service_created): use new
	transaction features to roll back removal of pending activation if
	we don't successfully create the service after all. Don't remove
	pending activation if the function fails.

	* dbus/dbus-list.c (_dbus_list_insert_before_link)
	(_dbus_list_insert_after_link): new code to facilitate
	services.c fixes

	* dbus/dbus-hash.c (_dbus_hash_table_insert_string_preallocated):
	new functionality, so we can preallocate the ability to insert
	into a hash table.

	* bus/connection.c (bus_transaction_add_cancel_hook): new function
	allowing us to put custom hooks in a transaction to be used for
	cancelling said transaction

	* doc/dbus-specification.sgml: add some discussion of secondary
	service owners, and disallow zero-length service names

	* bus/services.c (bus_registry_acquire_service): new function,
	splits out part of bus_driver_handle_acquire_service() and fixes
	a bug where we didn't remove the service doing the acquiring
	from the secondary queue if we failed to remove the current owner
	from the front of the queue.
parent 7074a246
2003-04-10 Havoc Pennington <hp@redhat.com>
* dbus/dbus-connection.c (dbus_connection_flush): don't spin on
the connection if it's disconnected
* bus/activation.c (bus_activation_service_created): use new
transaction features to roll back removal of pending activation if
we don't successfully create the service after all. Don't remove
pending activation if the function fails.
* dbus/dbus-list.c (_dbus_list_insert_before_link)
(_dbus_list_insert_after_link): new code to facilitate
services.c fixes
* dbus/dbus-hash.c (_dbus_hash_table_insert_string_preallocated):
new functionality, so we can preallocate the ability to insert
into a hash table.
* bus/connection.c (bus_transaction_add_cancel_hook): new function
allowing us to put custom hooks in a transaction to be used for
cancelling said transaction
* doc/dbus-specification.sgml: add some discussion of secondary
service owners, and disallow zero-length service names
* bus/services.c (bus_registry_acquire_service): new function,
splits out part of bus_driver_handle_acquire_service() and fixes
a bug where we didn't remove the service doing the acquiring
from the secondary queue if we failed to remove the current owner
from the front of the queue.
2003-04-10 Alexander Larsson <alexl@redhat.com>
* doc/dbus-specification.sgml:
......
......@@ -63,6 +63,7 @@ struct BusPendingActivationEntry
typedef struct
{
int refcount;
BusActivation *activation;
char *service_name;
DBusList *entries;
......@@ -94,13 +95,26 @@ handle_timeout_callback (DBusTimeout *timeout,
}
static void
bus_pending_activation_free (BusPendingActivation *pending_activation)
bus_pending_activation_ref (BusPendingActivation *pending_activation)
{
_dbus_assert (pending_activation->refcount > 0);
pending_activation->refcount += 1;
}
static void
bus_pending_activation_unref (BusPendingActivation *pending_activation)
{
DBusList *link;
if (pending_activation == NULL) /* hash table requires this */
return;
_dbus_assert (pending_activation->refcount > 0);
pending_activation->refcount -= 1;
if (pending_activation->refcount > 0)
return;
if (pending_activation->timeout_added)
{
_dbus_loop_remove_timeout (bus_context_get_loop (pending_activation->activation->context),
......@@ -396,7 +410,7 @@ bus_activation_new (BusContext *context,
}
activation->pending_activations = _dbus_hash_table_new (DBUS_HASH_STRING, NULL,
(DBusFreeFunction)bus_pending_activation_free);
(DBusFreeFunction)bus_pending_activation_unref);
if (activation->pending_activations == NULL)
{
......@@ -466,6 +480,75 @@ child_setup (void *data)
}
}
typedef struct
{
BusPendingActivation *pending_activation;
DBusPreallocatedHash *hash_entry;
} RestorePendingData;
static void
restore_pending (void *data)
{
RestorePendingData *d = data;
_dbus_assert (d->pending_activation != NULL);
_dbus_assert (d->hash_entry != NULL);
_dbus_verbose ("Restoring pending activation for service %s, has timeout = %d\n",
d->pending_activation->service_name,
d->pending_activation->timeout_added);
_dbus_hash_table_insert_string_preallocated (d->pending_activation->activation->pending_activations,
d->hash_entry,
d->pending_activation->service_name, d->pending_activation);
bus_pending_activation_ref (d->pending_activation);
d->hash_entry = NULL;
}
static void
free_pending_restore_data (void *data)
{
RestorePendingData *d = data;
if (d->hash_entry)
_dbus_hash_table_free_preallocated_entry (d->pending_activation->activation->pending_activations,
d->hash_entry);
bus_pending_activation_unref (d->pending_activation);
dbus_free (d);
}
static dbus_bool_t
add_restore_pending_to_transaction (BusTransaction *transaction,
BusPendingActivation *pending_activation)
{
RestorePendingData *d;
d = dbus_new (RestorePendingData, 1);
if (d == NULL)
return FALSE;
d->pending_activation = pending_activation;
d->hash_entry = _dbus_hash_table_preallocate_entry (d->pending_activation->activation->pending_activations);
bus_pending_activation_ref (d->pending_activation);
if (d->hash_entry == NULL ||
!bus_transaction_add_cancel_hook (transaction, restore_pending, d,
free_pending_restore_data))
{
free_pending_restore_data (d);
return FALSE;
}
_dbus_verbose ("Saved pending activation to be restored if the transaction fails\n");
return TRUE;
}
dbus_bool_t
bus_activation_service_created (BusActivation *activation,
const char *service_name,
......@@ -521,13 +604,19 @@ bus_activation_service_created (BusActivation *activation,
link = next;
}
if (!add_restore_pending_to_transaction (transaction, pending_activation))
{
_dbus_verbose ("Could not add cancel hook to transaction to revert removing pending activation\n");
BUS_SET_OOM (error);
goto error;
}
_dbus_hash_table_remove_string (activation->pending_activations, service_name);
return TRUE;
error:
_dbus_hash_table_remove_string (activation->pending_activations, service_name);
return FALSE;
}
......@@ -785,12 +874,13 @@ bus_activation_activate_service (BusActivation *activation,
}
pending_activation->activation = activation;
pending_activation->refcount = 1;
pending_activation->service_name = _dbus_strdup (service_name);
if (!pending_activation->service_name)
{
BUS_SET_OOM (error);
bus_pending_activation_free (pending_activation);
bus_pending_activation_unref (pending_activation);
bus_pending_activation_entry_free (pending_activation_entry);
return FALSE;
}
......@@ -803,7 +893,7 @@ bus_activation_activate_service (BusActivation *activation,
if (!pending_activation->timeout)
{
BUS_SET_OOM (error);
bus_pending_activation_free (pending_activation);
bus_pending_activation_unref (pending_activation);
bus_pending_activation_entry_free (pending_activation_entry);
return FALSE;
}
......@@ -815,7 +905,7 @@ bus_activation_activate_service (BusActivation *activation,
NULL))
{
BUS_SET_OOM (error);
bus_pending_activation_free (pending_activation);
bus_pending_activation_unref (pending_activation);
bus_pending_activation_entry_free (pending_activation_entry);
return FALSE;
}
......@@ -825,7 +915,7 @@ bus_activation_activate_service (BusActivation *activation,
if (!_dbus_list_append (&pending_activation->entries, pending_activation_entry))
{
BUS_SET_OOM (error);
bus_pending_activation_free (pending_activation);
bus_pending_activation_unref (pending_activation);
bus_pending_activation_entry_free (pending_activation_entry);
return FALSE;
}
......@@ -834,7 +924,7 @@ bus_activation_activate_service (BusActivation *activation,
pending_activation->service_name, pending_activation))
{
BUS_SET_OOM (error);
bus_pending_activation_free (pending_activation);
bus_pending_activation_unref (pending_activation);
return FALSE;
}
}
......
......@@ -139,6 +139,8 @@ bus_connection_disconnected (DBusConnection *connection)
if (!bus_service_remove_owner (service, connection,
transaction, &error))
{
_DBUS_ASSERT_ERROR_IS_SET (&error);
if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
{
dbus_error_free (&error);
......@@ -147,7 +149,11 @@ bus_connection_disconnected (DBusConnection *connection)
goto retry;
}
else
_dbus_assert_not_reached ("Removing service owner failed for non-memory-related reason");
{
_dbus_verbose ("Failed to remove service owner: %s %s\n",
error.name, error.message);
_dbus_assert_not_reached ("Removing service owner failed for non-memory-related reason");
}
}
bus_transaction_execute_and_free (transaction);
......@@ -746,19 +752,31 @@ bus_connection_send_oom_error (DBusConnection *connection,
d->oom_preallocated = NULL;
}
dbus_bool_t
bus_connection_add_owned_service (DBusConnection *connection,
BusService *service)
void
bus_connection_add_owned_service_link (DBusConnection *connection,
DBusList *link)
{
BusConnectionData *d;
d = BUS_CONNECTION_DATA (connection);
_dbus_assert (d != NULL);
if (!_dbus_list_append (&d->services_owned,
service))
_dbus_list_append_link (&d->services_owned, link);
}
dbus_bool_t
bus_connection_add_owned_service (DBusConnection *connection,
BusService *service)
{
DBusList *link;
link = _dbus_list_alloc_link (service);
if (link == NULL)
return FALSE;
bus_connection_add_owned_service_link (connection, link);
return TRUE;
}
......@@ -805,6 +823,13 @@ bus_connection_get_name (DBusConnection *connection)
return d->name;
}
/**
* Transactions
*
* Note that this is fairly fragile; in particular, don't try to use
* one transaction across any main loop iterations.
*/
typedef struct
{
BusTransaction *transaction;
......@@ -812,10 +837,18 @@ typedef struct
DBusPreallocatedSend *preallocated;
} MessageToSend;
typedef struct
{
BusTransactionCancelFunction cancel_function;
DBusFreeFunction free_data_function;
void *data;
} CancelHook;
struct BusTransaction
{
DBusList *connections;
BusContext *context;
DBusList *cancel_hooks;
};
static void
......@@ -831,6 +864,39 @@ message_to_send_free (DBusConnection *connection,
dbus_free (to_send);
}
static void
cancel_hook_cancel (void *element,
void *data)
{
CancelHook *ch = element;
_dbus_verbose ("Running transaction cancel hook\n");
if (ch->cancel_function)
(* ch->cancel_function) (ch->data);
}
static void
cancel_hook_free (void *element,
void *data)
{
CancelHook *ch = element;
if (ch->free_data_function)
(* ch->free_data_function) (ch->data);
dbus_free (ch);
}
static void
free_cancel_hooks (BusTransaction *transaction)
{
_dbus_list_foreach (&transaction->cancel_hooks,
cancel_hook_free, NULL);
_dbus_list_clear (&transaction->cancel_hooks);
}
BusTransaction*
bus_transaction_new (BusContext *context)
{
......@@ -980,6 +1046,11 @@ bus_transaction_cancel_and_free (BusTransaction *transaction)
_dbus_assert (transaction->connections == NULL);
_dbus_list_foreach (&transaction->cancel_hooks,
cancel_hook_cancel, NULL);
free_cancel_hooks (transaction);
dbus_free (transaction);
}
......@@ -1036,6 +1107,8 @@ bus_transaction_execute_and_free (BusTransaction *transaction)
_dbus_assert (transaction->connections == NULL);
free_cancel_hooks (transaction);
dbus_free (transaction);
}
......@@ -1090,3 +1163,31 @@ bus_transaction_send_error_reply (BusTransaction *transaction,
return TRUE;
}
dbus_bool_t
bus_transaction_add_cancel_hook (BusTransaction *transaction,
BusTransactionCancelFunction cancel_function,
void *data,
DBusFreeFunction free_data_function)
{
CancelHook *ch;
ch = dbus_new (CancelHook, 1);
if (ch == NULL)
return FALSE;
ch->cancel_function = cancel_function;
ch->data = data;
ch->free_data_function = free_data_function;
/* It's important that the hooks get run in reverse order that they
* were added
*/
if (!_dbus_list_prepend (&transaction->cancel_hooks, ch))
{
dbus_free (ch);
return FALSE;
}
return TRUE;
}
......@@ -25,6 +25,7 @@
#define BUS_CONNECTION_H
#include <dbus/dbus.h>
#include <dbus/dbus-list.h>
#include "bus.h"
typedef dbus_bool_t (* BusConnectionForeachFunction) (DBusConnection *connection,
......@@ -53,10 +54,13 @@ void bus_connection_send_oom_error (DBusConnection *connection,
DBusMessage *in_reply_to);
/* called by services.c */
dbus_bool_t bus_connection_add_owned_service (DBusConnection *connection,
BusService *service);
void bus_connection_remove_owned_service (DBusConnection *connection,
BusService *service);
dbus_bool_t bus_connection_add_owned_service (DBusConnection *connection,
BusService *service);
void bus_connection_remove_owned_service (DBusConnection *connection,
BusService *service);
void bus_connection_add_owned_service_link (DBusConnection *connection,
DBusList *link);
/* called by driver.c */
dbus_bool_t bus_connection_set_name (DBusConnection *connection,
......@@ -74,18 +78,24 @@ dbus_bool_t bus_connection_get_groups (DBusConnection *connection,
BusPolicy* bus_connection_get_policy (DBusConnection *connection);
/* transaction API so we can send or not send a block of messages as a whole */
BusTransaction* bus_transaction_new (BusContext *context);
BusContext* bus_transaction_get_context (BusTransaction *transaction);
BusConnections* bus_transaction_get_connections (BusTransaction *transaction);
dbus_bool_t bus_transaction_send_message (BusTransaction *transaction,
DBusConnection *connection,
DBusMessage *message);
dbus_bool_t bus_transaction_send_error_reply (BusTransaction *transaction,
DBusConnection *connection,
const DBusError *error,
DBusMessage *in_reply_to);
void bus_transaction_cancel_and_free (BusTransaction *transaction);
void bus_transaction_execute_and_free (BusTransaction *transaction);
typedef void (* BusTransactionCancelFunction) (void *data);
BusTransaction* bus_transaction_new (BusContext *context);
BusContext* bus_transaction_get_context (BusTransaction *transaction);
BusConnections* bus_transaction_get_connections (BusTransaction *transaction);
dbus_bool_t bus_transaction_send_message (BusTransaction *transaction,
DBusConnection *connection,
DBusMessage *message);
dbus_bool_t bus_transaction_send_error_reply (BusTransaction *transaction,
DBusConnection *connection,
const DBusError *error,
DBusMessage *in_reply_to);
void bus_transaction_cancel_and_free (BusTransaction *transaction);
void bus_transaction_execute_and_free (BusTransaction *transaction);
dbus_bool_t bus_transaction_add_cancel_hook (BusTransaction *transaction,
BusTransactionCancelFunction cancel_function,
void *data,
DBusFreeFunction free_data_function);
#endif /* BUS_CONNECTION_H */
......@@ -1442,6 +1442,47 @@ check_send_exit_to_service (BusContext *context,
return retval;
}
static dbus_bool_t
check_got_error (BusContext *context,
DBusConnection *connection,
const char *error_name)
{
DBusMessage *message;
dbus_bool_t retval;
retval = FALSE;
message = pop_message_waiting_for_memory (connection);
if (message == NULL)
{
_dbus_warn ("Did not get an expected error\n");
goto out;
}
if (!dbus_message_get_is_error (message))
{
_dbus_warn ("Expected an error, got %s\n",
dbus_message_get_name (message));
goto out;
}
if (!dbus_message_name_is (message, error_name))
{
_dbus_warn ("Expected error %s, got %s instead\n",
error_name,
dbus_message_get_name (message));
goto out;
}
retval = TRUE;
out:
if (message)
dbus_message_unref (message);
return retval;
}
#define EXISTENT_SERVICE_NAME "org.freedesktop.DBus.TestSuiteEchoService"
/* returns TRUE if the correct thing happens,
......@@ -1551,6 +1592,7 @@ check_existent_service_activation (BusContext *context,
else
{
dbus_bool_t got_service_deleted;
dbus_bool_t got_error;
if (!check_base_service_activated (context, connection,
message, &base_service))
......@@ -1570,9 +1612,22 @@ check_existent_service_activation (BusContext *context,
}
got_service_deleted = dbus_message_name_is (message, DBUS_MESSAGE_SERVICE_DELETED);
got_error = dbus_message_get_is_error (message);
dbus_connection_return_message (connection, message);
message = NULL;
if (got_error)
{
if (!check_got_error (context, connection,
DBUS_ERROR_SPAWN_CHILD_EXITED))
goto out;
/* A service deleted should be coming along now after this error.
* We can also get the error *after* the service deleted.
*/
got_service_deleted = TRUE;
}
if (got_service_deleted)
{
......@@ -1589,34 +1644,19 @@ check_existent_service_activation (BusContext *context,
if (csdd.failed)
goto out;
/* Now we should get an error about the service exiting */
block_connection_until_message_from_bus (context, connection);
/* and process everything again */
bus_test_run_everything (context);
message = pop_message_waiting_for_memory (connection);
if (message == NULL)
{
_dbus_warn ("Did not get an error from the service %s exiting\n",
EXISTENT_SERVICE_NAME);
goto out;
}
if (!dbus_message_get_is_error (message))
{
_dbus_warn ("Expected an error due to service exiting, got %s\n",
dbus_message_get_name (message));
goto out;
}
if (!dbus_message_name_is (message,
DBUS_ERROR_SPAWN_CHILD_EXITED))
/* Now we should get an error about the service exiting
* if we didn't get it before.
*/
if (!got_error)
{
_dbus_warn ("Expected error %s on service exit, got %s instead\n",
DBUS_ERROR_SPAWN_CHILD_EXITED,
dbus_message_get_name (message));
goto out;
block_connection_until_message_from_bus (context, connection);
/* and process everything again */
bus_test_run_everything (context);
if (!check_got_error (context, connection,
DBUS_ERROR_SPAWN_CHILD_EXITED))
goto out;
}
}
else
......@@ -1785,7 +1825,7 @@ bus_dispatch_test (const DBusString *test_data_dir)
if (!check_hello_message (context, baz))
_dbus_assert_not_reached ("hello message failed");
#if 0
#if 1
check2_try_iterations (context, foo, "existent_service_activation",
check_existent_service_activation);
#endif
......
......@@ -438,13 +438,10 @@ bus_driver_handle_acquire_service (DBusConnection *connection,
{
DBusMessage *reply;
DBusString service_name;
BusService *service;
char *name;
int service_reply;
int flags;
dbus_bool_t retval;
DBusConnection *old_owner;
DBusConnection *current_owner;
BusRegistry *registry;
_DBUS_ASSERT_ERROR_IS_CLEAR (error);
......@@ -461,27 +458,14 @@ bus_driver_handle_acquire_service (DBusConnection *connection,
retval = FALSE;
reply = NULL;
if (*name == ':')
{
/* Not allowed; only base services can start with ':' */
dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
"Cannot acquire a service starting with ':' such as \"%s\"",
name);
_dbus_verbose ("Attempt to acquire invalid base service name \"%s\"", name);
goto out;
}
_dbus_string_init_const (&service_name, name);
service = bus_registry_lookup (registry, &service_name);
if (service != NULL)
old_owner = bus_service_get_primary_owner (service);
else
old_owner = NULL;
if (!bus_registry_acquire_service (registry, connection,
&service_name, flags,
&service_reply, transaction,
error))
goto out;
reply = dbus_message_new_reply (message);
if (reply == NULL)
......@@ -495,67 +479,8 @@ bus_driver_handle_acquire_service (DBusConnection *connection,
BUS_SET_OOM (error);
goto out;
}
if (service == NULL)
{
service = bus_registry_ensure (registry,
&service_name, connection, transaction, error);
if (service == NULL)
goto out;
}
current_owner = bus_service_get_primary_owner (service);
if (old_owner == NULL)
{