diff options
author | Mattias Jernberg <mattiasj@axis.com> | 2019-07-11 18:13:46 +0200 |
---|---|---|
committer | Lennart Poettering <lennart@poettering.net> | 2019-08-14 16:12:31 +0200 |
commit | a5a8776ae5e4244b7f5acb2a1bfbe6e0b4d8a870 (patch) | |
tree | 0ba218a90b86b7fb13ad1bb0e3741d7c5c361439 /src/core | |
parent | f364a17dd165b398bd15470e93726bf6ca90be90 (diff) | |
download | systemd-a5a8776ae5e4244b7f5acb2a1bfbe6e0b4d8a870.tar.gz |
core: Avoid race when starting dbus services
In high load scenarios it is possible for services to be started
before the NameOwnerChanged signal is properly installed.
Emulate a callback by also queuing a GetNameOwner when the match is
installed.
Fixes: #12956
Diffstat (limited to 'src/core')
-rw-r--r-- | src/core/dbus.c | 4 | ||||
-rw-r--r-- | src/core/service.c | 13 | ||||
-rw-r--r-- | src/core/unit.c | 56 | ||||
-rw-r--r-- | src/core/unit.h | 3 |
4 files changed, 63 insertions, 13 deletions
diff --git a/src/core/dbus.c b/src/core/dbus.c index a8ce9ac447..bbfad1be74 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -784,7 +784,7 @@ static int manager_dispatch_sync_bus_names(sd_event_source *es, void *userdata) * changed, so synthesize a name owner changed signal. */ if (!streq_ptr(unique, s->bus_name_owner)) - UNIT_VTABLE(u)->bus_name_owner_change(u, name, s->bus_name_owner, unique); + UNIT_VTABLE(u)->bus_name_owner_change(u, s->bus_name_owner, unique); } else { /* So, the name we're watching is not on the bus. * This either means it simply hasn't appeared yet, @@ -793,7 +793,7 @@ static int manager_dispatch_sync_bus_names(sd_event_source *es, void *userdata) * and synthesize a name loss signal in this case. */ if (s->bus_name_owner) - UNIT_VTABLE(u)->bus_name_owner_change(u, name, s->bus_name_owner, NULL); + UNIT_VTABLE(u)->bus_name_owner_change(u, s->bus_name_owner, NULL); } } diff --git a/src/core/service.c b/src/core/service.c index bfbfa4be65..d1893228ec 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4067,7 +4067,6 @@ static int service_get_timeout(Unit *u, usec_t *timeout) { static void service_bus_name_owner_change( Unit *u, - const char *name, const char *old_owner, const char *new_owner) { @@ -4075,17 +4074,15 @@ static void service_bus_name_owner_change( int r; assert(s); - assert(name); - assert(streq(s->bus_name, name)); assert(old_owner || new_owner); if (old_owner && new_owner) - log_unit_debug(u, "D-Bus name %s changed owner from %s to %s", name, old_owner, new_owner); + log_unit_debug(u, "D-Bus name %s changed owner from %s to %s", s->bus_name, old_owner, new_owner); else if (old_owner) - log_unit_debug(u, "D-Bus name %s no longer registered by %s", name, old_owner); + log_unit_debug(u, "D-Bus name %s no longer registered by %s", s->bus_name, old_owner); else - log_unit_debug(u, "D-Bus name %s now registered by %s", name, new_owner); + log_unit_debug(u, "D-Bus name %s now registered by %s", s->bus_name, new_owner); s->bus_name_good = !!new_owner; @@ -4118,11 +4115,11 @@ static void service_bus_name_owner_change( /* Try to acquire PID from bus service */ - r = sd_bus_get_name_creds(u->manager->api_bus, name, SD_BUS_CREDS_PID, &creds); + r = sd_bus_get_name_creds(u->manager->api_bus, s->bus_name, SD_BUS_CREDS_PID, &creds); if (r >= 0) r = sd_bus_creds_get_pid(creds, &pid); if (r >= 0) { - log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, name, pid); + log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, s->bus_name, pid); service_set_main_pid(s, pid); unit_watch_pid(UNIT(s), pid, false); diff --git a/src/core/unit.c b/src/core/unit.c index 00181941fd..9dc75223a7 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3242,7 +3242,46 @@ static int signal_name_owner_changed(sd_bus_message *message, void *userdata, sd new_owner = empty_to_null(new_owner); if (UNIT_VTABLE(u)->bus_name_owner_change) - UNIT_VTABLE(u)->bus_name_owner_change(u, name, old_owner, new_owner); + UNIT_VTABLE(u)->bus_name_owner_change(u, old_owner, new_owner); + + return 0; +} + +static int get_name_owner_handler(sd_bus_message *message, void *userdata, sd_bus_error *error) { + const sd_bus_error *e; + const char *new_owner; + Unit *u = userdata; + int r; + + assert(message); + assert(u); + + u->get_name_owner_slot = sd_bus_slot_unref(u->get_name_owner_slot); + + if (sd_bus_error_is_set(error)) { + log_error("Failed to get name owner from bus: %s", error->message); + return 0; + } + + e = sd_bus_message_get_error(message); + if (sd_bus_error_has_name(e, "org.freedesktop.DBus.Error.NameHasNoOwner")) + return 0; + + if (e) { + log_error("Unexpected error response from GetNameOwner: %s", e->message); + return 0; + } + + r = sd_bus_message_read(message, "s", &new_owner); + if (r < 0) { + bus_log_parse_error(r); + return 0; + } + + new_owner = empty_to_null(new_owner); + + if (UNIT_VTABLE(u)->bus_name_owner_change) + UNIT_VTABLE(u)->bus_name_owner_change(u, NULL, new_owner); return 0; } @@ -3264,7 +3303,19 @@ int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name) { "member='NameOwnerChanged'," "arg0='", name, "'"); - return sd_bus_add_match_async(bus, &u->match_bus_slot, match, signal_name_owner_changed, NULL, u); + int r = sd_bus_add_match_async(bus, &u->match_bus_slot, match, signal_name_owner_changed, NULL, u); + if (r < 0) + return r; + + return sd_bus_call_method_async(bus, + &u->get_name_owner_slot, + "org.freedesktop.DBus", + "/org/freedesktop/DBus", + "org.freedesktop.DBus", + "GetNameOwner", + get_name_owner_handler, + u, + "s", name); } int unit_watch_bus_name(Unit *u, const char *name) { @@ -3299,6 +3350,7 @@ void unit_unwatch_bus_name(Unit *u, const char *name) { (void) hashmap_remove_value(u->manager->watch_bus, name, u); u->match_bus_slot = sd_bus_slot_unref(u->match_bus_slot); + u->get_name_owner_slot = sd_bus_slot_unref(u->get_name_owner_slot); } bool unit_can_serialize(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index 47ec9877a6..9ad86fddf6 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -146,6 +146,7 @@ typedef struct Unit { /* The slot used for watching NameOwnerChanged signals */ sd_bus_slot *match_bus_slot; + sd_bus_slot *get_name_owner_slot; /* References to this unit from clients */ sd_bus_track *bus_track; @@ -528,7 +529,7 @@ typedef struct UnitVTable { void (*notify_message)(Unit *u, const struct ucred *ucred, char **tags, FDSet *fds); /* Called whenever a name this Unit registered for comes or goes away. */ - void (*bus_name_owner_change)(Unit *u, const char *name, const char *old_owner, const char *new_owner); + void (*bus_name_owner_change)(Unit *u, const char *old_owner, const char *new_owner); /* Called for each property that is being set */ int (*bus_set_property)(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); |