containers: Add named arguments to alter how the server is stopped

This adds the initial named arguments that we anticipate we will need
for Flatpak and Snap: Flatpak will want to hold the server open until the
xdg-dbus-proxy exits, while Snap developers want to clean up the
per-container servers explicitly and have their existence stored as part
of the persistent state of the restartable snapd service.

Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/186
Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie 2023-11-30 18:38:11 +00:00
parent 9a8d7fb90f
commit 5db4c852fa
3 changed files with 231 additions and 28 deletions

View file

@ -34,11 +34,13 @@
#endif
#include <sys/types.h>
#include <unistd.h>
#include "dbus/dbus-asv-util.h"
#include "dbus/dbus-hash.h"
#include "dbus/dbus-message-internal.h"
#include "dbus/dbus-sysdeps-unix.h"
#include "dbus/dbus-watch.h"
#include "connection.h"
#include "dispatch.h"
@ -64,8 +66,11 @@ typedef struct
/* List of owned DBusConnection, removed when the DBusConnection is
* removed from the bus */
DBusList *connections;
DBusWatch *stop_on_notify_watch;
unsigned long uid;
int stop_on_notify_fd;
unsigned announced:1;
unsigned stop_on_disconnect:1;
} BusContainerServer;
/* Data attached to a DBusConnection that has created container servers. */
@ -273,6 +278,29 @@ oom:
return FALSE;
}
static void
bus_container_server_remove_notify (BusContainerServer *self)
{
DBusWatch *watch;
/* Transfer the reference to this function */
watch = self->stop_on_notify_watch;
self->stop_on_notify_watch = NULL;
if (watch != NULL)
{
_dbus_loop_remove_watch (bus_context_get_loop (self->context), watch);
_dbus_watch_invalidate (watch);
_dbus_watch_unref (watch);
}
if (self->stop_on_notify_fd >= 0)
{
close (self->stop_on_notify_fd);
self->stop_on_notify_fd = -1;
}
}
static void
bus_container_server_unref (BusContainerServer *self)
{
@ -297,6 +325,8 @@ bus_container_server_unref (BusContainerServer *self)
self->announced = FALSE;
}
bus_container_server_remove_notify (self);
/* As long as the server is listening, the BusContainerServer can't
* be freed, because the DBusServer holds a reference to the
* BusContainerServer */
@ -360,6 +390,8 @@ bus_container_server_stop_listening (BusContainerServer *self)
/* In case the DBusServer holds the last reference to self */
bus_container_server_ref (self);
bus_container_server_remove_notify (self);
if (self->server != NULL)
{
dbus_server_set_new_connection_function (self->server, NULL, NULL, NULL);
@ -407,6 +439,8 @@ bus_container_server_new (BusContext *context,
self->containers = bus_containers_ref (containers);
self->server = NULL;
self->creator = dbus_connection_ref (creator);
self->stop_on_notify_fd = -1;
self->stop_on_disconnect = TRUE;
if (containers->next_container_id >=
DBUS_UINT64_CONSTANT (0xFFFFFFFFFFFFFFFF))
@ -672,6 +706,38 @@ bus_container_server_listen (BusContainerServer *self,
return TRUE;
}
static dbus_bool_t
check_named_parameter_type (DBusMessageIter *variant_iter,
const char *param_name,
int expected_type,
DBusError *error)
{
if (dbus_message_iter_get_arg_type (variant_iter) != expected_type)
{
dbus_set_error (error, DBUS_ERROR_INVALID_ARGS,
"Named parameter %s should have type '%c', not '%c'",
param_name, expected_type,
dbus_message_iter_get_arg_type (variant_iter));
return FALSE;
}
return TRUE;
}
static dbus_bool_t
bus_containers_handle_stop_notify (DBusWatch *watch,
unsigned int flags,
void *data)
{
BusContainerServer *server = data;
_dbus_assert (watch == server->stop_on_notify_watch);
_dbus_verbose ("Stopping %s because notify fd became readable",
server->path);
bus_container_server_stop_listening (server);
return TRUE;
}
dbus_bool_t
bus_containers_handle_add_server (DBusConnection *connection,
BusTransaction *transaction,
@ -825,9 +891,12 @@ bus_containers_handle_add_server (DBusConnection *connection,
_dbus_assert (dbus_message_iter_get_arg_type (&iter) == DBUS_TYPE_ARRAY);
dbus_message_iter_recurse (&iter, &dict_iter);
while (dbus_message_iter_get_arg_type (&dict_iter) != DBUS_TYPE_INVALID)
for (;
dbus_message_iter_get_arg_type (&dict_iter) != DBUS_TYPE_INVALID;
dbus_message_iter_next (&dict_iter))
{
DBusMessageIter pair_iter;
DBusMessageIter variant_iter;
const char *param_name;
_dbus_assert (dbus_message_iter_get_arg_type (&dict_iter) ==
@ -838,11 +907,64 @@ bus_containers_handle_add_server (DBusConnection *connection,
DBUS_TYPE_STRING);
dbus_message_iter_get_basic (&pair_iter, &param_name);
/* If we supported any named parameters, we'd copy them into the data
* structure here; but we don't, so fail instead. */
dbus_set_error (error, DBUS_ERROR_INVALID_ARGS,
"Named parameter %s is not understood", param_name);
goto fail;
if (!dbus_message_iter_next (&pair_iter))
_dbus_assert_not_reached ("dict entry with less than 2 items");
_dbus_assert (dbus_message_iter_get_arg_type (&pair_iter) ==
DBUS_TYPE_VARIANT);
dbus_message_iter_recurse (&pair_iter, &variant_iter);
if (strcmp (param_name, "StopOnDisconnect") == 0)
{
dbus_bool_t value;
if (!check_named_parameter_type (&variant_iter, param_name,
DBUS_TYPE_BOOLEAN, error))
goto fail;
dbus_message_iter_get_basic (&variant_iter, &value);
server->stop_on_disconnect = !!value;
}
else if (strcmp (param_name, "StopOnNotify") == 0)
{
if (!check_named_parameter_type (&variant_iter, param_name,
DBUS_TYPE_UNIX_FD, error))
goto fail;
dbus_message_iter_get_basic (&variant_iter,
&server->stop_on_notify_fd);
if (server->stop_on_notify_fd < 0)
{
dbus_set_error (error, DBUS_ERROR_INVALID_ARGS,
"Unable to retrieve StopOnNotify fd");
goto fail;
}
server->stop_on_notify_watch = _dbus_watch_new (server->stop_on_notify_fd,
DBUS_WATCH_READABLE,
TRUE, /* enabled */
bus_containers_handle_stop_notify,
server,
NULL);
if (server->stop_on_notify_watch == NULL ||
!_dbus_loop_add_watch (bus_context_get_loop (context),
server->stop_on_notify_watch))
{
BUS_SET_OOM (error);
goto fail;
}
}
else
{
dbus_set_error (error, DBUS_ERROR_INVALID_ARGS,
"Named parameter %s is not understood", param_name);
goto fail;
}
if (dbus_message_iter_next (&pair_iter))
_dbus_assert_not_reached ("dict entry with more than 2 items");
}
/* End of arguments */
@ -999,13 +1121,32 @@ dbus_bool_t
bus_containers_supported_arguments_getter (BusContext *server,
DBusMessageIter *var_iter)
{
/* Please keep these sorted in strcmp (LC_ALL=C sort) order */
static const char * const supported[] =
{
"StopOnDisconnect",
"StopOnNotify",
};
DBusMessageIter arr_iter;
size_t i;
/* There are none so far */
return dbus_message_iter_open_container (var_iter, DBUS_TYPE_ARRAY,
DBUS_TYPE_STRING_AS_STRING,
&arr_iter) &&
dbus_message_iter_close_container (var_iter, &arr_iter);
if (!dbus_message_iter_open_container (var_iter, DBUS_TYPE_ARRAY,
DBUS_TYPE_STRING_AS_STRING,
&arr_iter))
return FALSE;
for (i = 0; i < _DBUS_N_ELEMENTS (supported); i++)
{
if (!dbus_message_iter_append_basic (&arr_iter,
DBUS_TYPE_STRING,
&supported[i]))
{
dbus_message_iter_abandon_container (var_iter, &arr_iter);
return FALSE;
}
}
return dbus_message_iter_close_container (var_iter, &arr_iter);
}
dbus_bool_t
@ -1436,6 +1577,11 @@ bus_containers_remove_connection (BusContainers *self,
DBusList *iter;
DBusList *next;
_dbus_verbose ("Closing connection %s (%s) which has owned at least "
"one server",
bus_connection_get_name (connection),
bus_connection_get_loginfo (connection));
for (iter = _dbus_list_get_first_link (&creator_data->servers);
iter != NULL;
iter = next)
@ -1448,9 +1594,19 @@ bus_containers_remove_connection (BusContainers *self,
_dbus_assert (server->creator == connection);
/* This will invalidate iter and server if there are no open
* connections to this server */
bus_container_server_stop_listening (server);
if (server->stop_on_disconnect)
{
_dbus_verbose ("Stopping %s", server->path);
/* This will invalidate iter and server if there are no open
* connections to this server */
bus_container_server_stop_listening (server);
}
else
{
_dbus_verbose ("Not stopping %s because it was created with "
"StopOnDisconnect=FALSE",
server->path);
}
}
}

View file

@ -7619,7 +7619,7 @@
<para>
The last argument (the <firstterm>named arguments</firstterm>)
will be used in future versions of this specification to modify
can be used to modify
the behaviour of this method call; all keys in this dictionary not
containing <literal>.</literal> are reserved by this specification
for this purpose. It can also contain implementation-specific
@ -7630,6 +7630,60 @@
extension point.
</para>
<para>
The only named arguments that are allowed are the ones
listed in the
<literal>org.freedesktop.DBus.Containers1.SupportedArguments</literal>
property. All other named arguments are an error.
The possible keys currently defined for the named arguments are:
<informaltable>
<tgroup cols="4">
<thead>
<row>
<entry>Key</entry>
<entry>Value type</entry>
<entry>Default value</entry>
<entry>Meaning</entry>
</row>
</thead>
<tbody>
<row>
<entry>StopOnDisconnect</entry>
<entry>BOOLEAN</entry>
<entry>true</entry>
<entry>
If true or omitted, the server socket will cease to
listen for new connections (in the same way as if
<literal>StopListening</literal> had been called
successfully) when the caller of this method
disconnects from the bus. Any confined connections
that are already active are unaffected.
If false, the server socket continues to listen after
the caller disconnects, until there is some other reason
for it to stop.
</entry>
</row>
<row>
<entry>StopOnNotify</entry>
<entry>UNIX_FD</entry>
<entry>(none)</entry>
<entry>
A file descriptor which will become readable (either
data available or end-of-file) when it is time to stop
listening for new connections. In practice, this is
expected to be the read end of a Unix pipe, for which
the corresponding write end will be closed (either
explicitly or when its owning process exits) when the
container manager detects that the application has
stopped running. This will often be combined with
<literal>{ "StopOnDisconnect": false }</literal>.
</entry>
</row>
</tbody>
</tgroup>
</informaltable>
</para>
<para>
Keys in the returned result dictionary not containing "." are defined
by this specification. Bus daemon implementors wishing to emit
@ -7700,17 +7754,6 @@
any particular filtering applied to them by the message bus.
</para>
<para>
In the most basic form of a call to this method, the server
socket will cease to listen for new connections (in the same
way as if <literal>StopListening</literal> had been called
successfully) when the caller of this method disconnects from
the bus. Any confined connections that are already active are
unaffected. Future versions of this specification are likely
to provide a way for the caller to alter this behaviour by
specifying named arguments.
</para>
<para>
The container server object path remains valid for as
long as one or more confined connection via the same server

View file

@ -290,6 +290,7 @@ test_get_supported_arguments (Fixture *f,
GVariant *v;
#ifdef DBUS_ENABLE_CONTAINERS
const gchar **args;
gsize i;
gsize len;
#endif
@ -311,8 +312,11 @@ test_get_supported_arguments (Fixture *f,
g_assert_cmpstr (g_variant_get_type_string (v), ==, "as");
args = g_variant_get_strv (v, &len);
/* No arguments are defined yet */
g_assert_cmpuint (len, ==, 0);
g_assert_cmpuint (len, ==, 2);
i = 0;
g_assert_cmpstr (args[i++], ==, "StopOnDisconnect");
g_assert_cmpstr (args[i++], ==, "StopOnNotify");
g_assert_cmpstr (args[i++], ==, NULL);
g_free (args);
g_variant_unref (v);