Skip to content

Commit

Permalink
pid1: lookup owning PID of BusName= name of services asynchronously
Browse files Browse the repository at this point in the history
A first step of removing blocking calls to the D-Bus broker from PID 1.
There's a lot more to got (i.e. grep src/core/ for sd_bus_creds
basically), but it's a start.

Removing blocking calls to D-Bus broker deals systematicallly with
deadlocks caused by dbus-daemon blocking on synchronous IPC calls back
to PID1 (e.g. Varlink calls through nss-systemd). Bugs such as #15316.

Also-see: systemd/systemd#22038 (comment)
(cherry picked from commit e39eb04)
  • Loading branch information
poettering authored and bluca committed Mar 5, 2022
1 parent a51e540 commit cf39014
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 19 deletions.
91 changes: 72 additions & 19 deletions src/core/service.c
Expand Up @@ -398,6 +398,8 @@ static void service_done(Unit *u) {
s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source);
s->exec_fd_event_source = sd_event_source_disable_unref(s->exec_fd_event_source);

s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);

service_release_resources(u);
}

Expand Down Expand Up @@ -4233,6 +4235,60 @@ static int service_get_timeout(Unit *u, usec_t *timeout) {
return 1;
}

static bool pick_up_pid_from_bus_name(Service *s) {
assert(s);

/* If the service is running but we have no main PID yet, get it from the owner of the D-Bus name */

return !pid_is_valid(s->main_pid) &&
IN_SET(s->state,
SERVICE_START,
SERVICE_START_POST,
SERVICE_RUNNING,
SERVICE_RELOAD);
}

static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, sd_bus_error *ret_error) {
const sd_bus_error *e;
Unit *u = userdata;
uint32_t pid;
Service *s;
int r;

assert(reply);
assert(u);

s = SERVICE(u);
s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);

if (!s->bus_name || !pick_up_pid_from_bus_name(s))
return 1;

e = sd_bus_message_get_error(reply);
if (e) {
r = sd_bus_error_get_errno(e);
log_warning_errno(r, "GetConnectionUnixProcessID() failed: %s", bus_error_message(e, r));
return 1;
}

r = sd_bus_message_read(reply, "u", &pid);
if (r < 0) {
bus_log_parse_error(r);
return 1;
}

if (!pid_is_valid(pid)) {
log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "GetConnectionUnixProcessID() returned invalid PID");
return 1;
}

log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, s->bus_name, (pid_t) pid);

service_set_main_pid(s, pid);
unit_watch_pid(UNIT(s), pid, false);
return 1;
}

static void service_bus_name_owner_change(Unit *u, const char *new_owner) {

Service *s = SERVICE(u);
Expand Down Expand Up @@ -4263,28 +4319,25 @@ static void service_bus_name_owner_change(Unit *u, const char *new_owner) {
else if (s->state == SERVICE_START && new_owner)
service_enter_start_post(s);

} else if (new_owner &&
s->main_pid <= 0 &&
IN_SET(s->state,
SERVICE_START,
SERVICE_START_POST,
SERVICE_RUNNING,
SERVICE_RELOAD)) {

_cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
pid_t pid;
} else if (new_owner && pick_up_pid_from_bus_name(s)) {

/* Try to acquire PID from bus service */

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, s->bus_name, pid);

service_set_main_pid(s, pid);
unit_watch_pid(UNIT(s), pid, false);
}
s->bus_name_pid_lookup_slot = sd_bus_slot_unref(s->bus_name_pid_lookup_slot);

r = sd_bus_call_method_async(
u->manager->api_bus,
&s->bus_name_pid_lookup_slot,
"org.freedesktop.DBus",
"/org/freedesktop/DBus",
"org.freedesktop.DBus",
"GetConnectionUnixProcessID",
bus_name_pid_lookup_callback,
s,
"s",
s->bus_name);
if (r < 0)
log_debug_errno(r, "Failed to request owner PID of service name, ignoring: %m");
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/core/service.h
Expand Up @@ -195,6 +195,8 @@ struct Service {
NotifyAccess notify_access;
NotifyState notify_state;

sd_bus_slot *bus_name_pid_lookup_slot;

sd_event_source *exec_fd_event_source;

ServiceFDStore *fd_store;
Expand Down

0 comments on commit cf39014

Please sign in to comment.