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.

3 files changed, 67 insertions, 12 deletions

diff --git a/src/bus-proxyd/driver.c b/src/bus-proxyd/driver.c
index aa94c6f51b..133454ddbf 100644
--- 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 c37b09b9c0..b3ec048d03 100644
--- 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 ccb951c109..6aac650ac9 100644
--- 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);