bus-proxy: make StartServiceByName synchronous

The StartServiceByName() call was provided by dbus-daemon to activate a
service without sending a message. On receiption, dbus-daemon schedules
an activation request (different modes are supported) and sends back the
reply once activation is done.

With kdbus, we marked StartServiceByName() as deprecated. There is no
real reason to start services explicitly. Instead, applications should
just *use* the service and rely on it being activated implicitly.
However, we provide compatibility with dbus-daemon and implement
StartServiceByName() on the proxy via a call to
org.freedesktop.DBus.Peer.Ping() on the destination. This will activate
the peer implicitly as part of the no-op Ping() method call (regardless
whether the peer actually implements that call).

Now, the problem is, StartServiceByName() was synchronous on dbus-daemon
but isn't on bus-proxy. Hence, on return, there is no guarantee that
ListNames includes the activated name. As this is required by some
applications, we need to make this synchronous.

This patch makes the proxy track the Ping() method call and send the
reply of StartServiceByName() only once Ping() returned. We do not look
at possible errors of Ping(), as there is no strict requirement for the
peer to implement org.freedesktop.DBus.Peer. Furthermore, any interesting
error should have already been caught by sd_bus_send() before.

Note:
        This race was triggered by gdbus. The gdbus-proxy implementation
        relies on a name to be available after StartServiceByName()
        returns. This is highly fragile and should be dropped by gdbus.
        Even if the call is synchronous, there is no reason whatsoever to
        assume the service did not exit-on-idle before ListNames()
        returns.
        However, this race is much less likely than the startup race, so
        we try to be compatible to dbus-daemon now.

diff --git a/src/bus-proxyd/driver.c b/src/bus-proxyd/driver.c
index aa94c6f51be0dd59b8cf706072ffd045b22fb91c..133454ddbf5bd470a860dd2484fdbc210ee01236 100644 (file)
--- a/src/bus-proxyd/driver.c
+++ b/src/bus-proxyd/driver.c
@@ -71,6 +71,27 @@ static int get_creds_by_message(sd_bus *bus, sd_bus_message *m, uint64_t mask, s
         return get_creds_by_name(bus, name, mask, _creds, error);
 }
 
+static int driver_activation(sd_bus_message *reply, void *userdata, sd_bus_error *error) {
+        _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        ProxyActivation *activation = userdata;
+
+        /*
+         * The org.freedesktop.DBus.Peer.Ping() call returned. We don't care
+         * whether this succeeded, failed, was not implemented or timed out. We
+         * cannot assume that the target reacts to this properly. Hence, just
+         * send the reply to the activation request and be done.
+         */
+
+        m = activation->request; /* claim reference */
+
+        --activation->proxy->n_activations;
+        LIST_REMOVE(activations_by_proxy, activation->proxy->activations, activation);
+        sd_bus_slot_unref(activation->slot);
+        free(activation);
+
+        return synthetic_reply_method_return(m, "u", BUS_START_REPLY_SUCCESS);
+}
+
 int bus_proxy_process_driver(Proxy *p, sd_bus *a, sd_bus *b, sd_bus_message *m, SharedPolicy *sp, const struct ucred *ucred, Set *owned_names) {
         int r;
 
@@ -587,6 +608,7 @@ int bus_proxy_process_driver(Proxy *p, sd_bus *a, sd_bus *b, sd_bus_message *m,
 
         } else if (sd_bus_message_is_method_call(m, "org.freedesktop.DBus", "StartServiceByName")) {
                 _cleanup_bus_message_unref_ sd_bus_message *msg = NULL;
+                ProxyActivation *activation;
                 const char *name;
                 uint32_t flags;
 
@@ -606,21 +628,32 @@ int bus_proxy_process_driver(Proxy *p, sd_bus *a, sd_bus *b, sd_bus_message *m,
                 if (r != -ESRCH)
                         return synthetic_reply_method_errno(m, r, NULL);
 
-                r = sd_bus_message_new_method_call(
-                                a,
-                                &msg,
-                                name,
-                                "/",
-                                "org.freedesktop.DBus.Peer",
-                                "Ping");
-                if (r < 0)
-                        return synthetic_reply_method_errno(m, r, NULL);
+                if (p->n_activations >= PROXY_ACTIVATIONS_MAX)
+                        return synthetic_reply_method_errno(m, -EMFILE, NULL);
 
-                r = sd_bus_send(a, msg, NULL);
-                if (r < 0)
+                activation = new0(ProxyActivation, 1);
+                if (!activation)
+                        return synthetic_reply_method_errno(m, -ENOMEM, NULL);
+
+                r = sd_bus_call_method_async(a,
+                                             &activation->slot,
+                                             name,
+                                             "/",
+                                             "org.freedesktop.DBus.Peer",
+                                             "Ping",
+                                             driver_activation,
+                                             activation,
+                                             NULL);
+                if (r < 0) {
+                        free(activation);
                         return synthetic_reply_method_errno(m, r, NULL);
+                }
 
-                return synthetic_reply_method_return(m, "u", BUS_START_REPLY_SUCCESS);
+                activation->proxy = p;
+                activation->request = sd_bus_message_ref(m);
+                LIST_PREPEND(activations_by_proxy, p->activations, activation);
+                ++p->n_activations;
+                return 1;
 
         } else if (sd_bus_message_is_method_call(m, "org.freedesktop.DBus", "UpdateActivationEnvironment")) {
                 _cleanup_bus_message_unref_ sd_bus_message *msg = NULL;

diff --git a/src/bus-proxyd/proxy.c b/src/bus-proxyd/proxy.c
index c37b09b9c0ad33ea5b83919365fa5e8207684c1e..b3ec048d03a39bd9cf1a30b02ae862bc7f10c72f 100644 (file)
--- a/src/bus-proxyd/proxy.c
+++ b/src/bus-proxyd/proxy.c
@@ -261,9 +261,18 @@ int proxy_new(Proxy **out, int in_fd, int out_fd, const char *destination) {
 }
 
 Proxy *proxy_free(Proxy *p) {
+        ProxyActivation *activation;
+
         if (!p)
                 return NULL;
 
+        while ((activation = p->activations)) {
+                LIST_REMOVE(activations_by_proxy, p->activations, activation);
+                sd_bus_message_unref(activation->request);
+                sd_bus_slot_unref(activation->slot);
+                free(activation);
+        }
+
         sd_bus_flush_close_unref(p->local_bus);
         sd_bus_flush_close_unref(p->destination_bus);
         set_free_free(p->owned_names);

diff --git a/src/bus-proxyd/proxy.h b/src/bus-proxyd/proxy.h
index ccb951c1092cc00e8b1d323721bcbd9e73c8a9fe..6aac650ac93a8e469e3cadfc97f51d34b756a99b 100644 (file)
--- a/src/bus-proxyd/proxy.h
+++ b/src/bus-proxyd/proxy.h
@@ -25,6 +25,9 @@
 #include "bus-xml-policy.h"
 
 typedef struct Proxy Proxy;
+typedef struct ProxyActivation ProxyActivation;
+
+#define PROXY_ACTIVATIONS_MAX (16) /* max parallel activation requests */
 
 struct Proxy {
         sd_bus *local_bus;
@@ -37,12 +40,22 @@ struct Proxy {
         Set *owned_names;
         SharedPolicy *policy;
 
+        LIST_HEAD(ProxyActivation, activations);
+        size_t n_activations;
+
         bool got_hello : 1;
         bool queue_overflow : 1;
         bool message_matched : 1;
         bool synthetic_matched : 1;
 };
 
+struct ProxyActivation {
+        LIST_FIELDS(ProxyActivation, activations_by_proxy);
+        Proxy *proxy;
+        sd_bus_message *request;
+        sd_bus_slot *slot;
+};
+
 int proxy_new(Proxy **out, int in_fd, int out_fd, const char *dest);
 Proxy *proxy_free(Proxy *p);