diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 3927d6df850..adc3e4ab2dd 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -2040,7 +2040,7 @@ static int on_cgroup_empty_event(sd_event_source *s, void *userdata) { /* More stuff queued, let's make sure we remain enabled */ r = sd_event_source_set_enabled(s, SD_EVENT_ONESHOT); if (r < 0) - log_debug_errno(r, "Failed to reenable cgroup empty event source: %m"); + log_debug_errno(r, "Failed to reenable cgroup empty event source, ignoring: %m"); } unit_add_to_gc_queue(u); diff --git a/src/core/scope.c b/src/core/scope.c index 7410f154e6c..5db32960441 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -92,8 +92,10 @@ static void scope_set_state(Scope *s, ScopeState state) { if (!IN_SET(state, SCOPE_STOP_SIGTERM, SCOPE_STOP_SIGKILL)) s->timer_event_source = sd_event_source_unref(s->timer_event_source); - if (IN_SET(state, SCOPE_DEAD, SCOPE_FAILED)) + if (IN_SET(state, SCOPE_DEAD, SCOPE_FAILED)) { unit_unwatch_all_pids(UNIT(s)); + unit_dequeue_rewatch_pids(UNIT(s)); + } if (state != old_state) log_debug("%s changed %s -> %s", UNIT(s)->id, scope_state_to_string(old_state), scope_state_to_string(state)); @@ -212,7 +214,7 @@ static int scope_coldplug(Unit *u) { } if (!IN_SET(s->deserialized_state, SCOPE_DEAD, SCOPE_FAILED)) - unit_watch_all_pids(UNIT(s)); + (void) unit_enqueue_rewatch_pids(u); bus_scope_track_controller(s); @@ -257,7 +259,12 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) { if (s->result == SCOPE_SUCCESS) s->result = f; - unit_watch_all_pids(UNIT(s)); + /* Before sending any signal, make sure we track all members of this cgroup */ + (void) unit_watch_all_pids(UNIT(s)); + + /* Also, enqueue a job that we recheck all our PIDs a bit later, given that it's likely some processes have + * died now */ + (void) unit_enqueue_rewatch_pids(UNIT(s)); /* If we have a controller set let's ask the controller nicely to terminate the scope, instead of us going * directly into SIGTERM berserk mode */ @@ -340,6 +347,9 @@ static int scope_start(Unit *u) { s->result = SCOPE_SUCCESS; scope_set_state(s, SCOPE_RUNNING); + + /* Start watching the PIDs currently in the scope */ + (void) unit_enqueue_rewatch_pids(UNIT(s)); return 1; } @@ -455,17 +465,13 @@ static void scope_notify_cgroup_empty_event(Unit *u) { } static void scope_sigchld_event(Unit *u, pid_t pid, int code, int status) { - assert(u); /* If we get a SIGCHLD event for one of the processes we were interested in, then we look for others to * watch, under the assumption that we'll sooner or later get a SIGCHLD for them, as the original * process we watched was probably the parent of them, and they are hence now our children. */ - unit_tidy_watch_pids(u, 0, 0); - unit_watch_all_pids(u); - /* If the PID set is empty now, then let's finish this off. */ - unit_synthesize_cgroup_empty_event(u); + (void) unit_enqueue_rewatch_pids(u); } static int scope_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata) { @@ -515,13 +521,9 @@ int scope_abandon(Scope *s) { scope_set_state(s, SCOPE_ABANDONED); - /* The client is no longer watching the remaining processes, - * so let's step in here, under the assumption that the - * remaining processes will be sooner or later reassigned to - * us as parent. */ - - unit_tidy_watch_pids(UNIT(s), 0, 0); - unit_watch_all_pids(UNIT(s)); + /* The client is no longer watching the remaining processes, so let's step in here, under the assumption that + * the remaining processes will be sooner or later reassigned to us as parent. */ + (void) unit_enqueue_rewatch_pids(UNIT(s)); return 0; } diff --git a/src/core/service.c b/src/core/service.c index bd62fbce3a0..db0c1dc397a 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1058,8 +1058,10 @@ static void service_set_state(Service *s, ServiceState state) { s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID; } - if (IN_SET(state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_AUTO_RESTART)) + if (IN_SET(state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_AUTO_RESTART)) { unit_unwatch_all_pids(UNIT(s)); + unit_dequeue_rewatch_pids(UNIT(s)); + } if (!IN_SET(state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, @@ -1154,17 +1156,15 @@ static int service_coldplug(Unit *u) { return r; } - if (!IN_SET(s->deserialized_state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_AUTO_RESTART)) - unit_watch_all_pids(UNIT(s)); - - if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) - service_start_watchdog(s); - if (!IN_SET(s->deserialized_state, SERVICE_DEAD, SERVICE_FAILED, SERVICE_AUTO_RESTART)) { + (void) unit_enqueue_rewatch_pids(u); (void) unit_setup_dynamic_creds(u); (void) unit_setup_exec_runtime(u); } + if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) + service_start_watchdog(s); + if (UNIT_ISSET(s->accept_socket)) { Socket* socket = SOCKET(UNIT_DEREF(s->accept_socket)); @@ -1679,7 +1679,8 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { s->result = f; service_unwatch_control_pid(s); - unit_watch_all_pids(UNIT(s)); + + (void) unit_enqueue_rewatch_pids(UNIT(s)); s->control_command = s->exec_command[SERVICE_EXEC_STOP_POST]; if (s->control_command) { @@ -1731,7 +1732,12 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f if (s->result == SERVICE_SUCCESS) s->result = f; - unit_watch_all_pids(UNIT(s)); + /* Before sending any signal, make sure we track all members of this cgroup */ + (void) unit_watch_all_pids(UNIT(s)); + + /* Also, enqueue a job that we recheck all our PIDs a bit later, given that it's likely some processes have + * died now */ + (void) unit_enqueue_rewatch_pids(UNIT(s)); r = unit_kill_context( UNIT(s), @@ -1772,7 +1778,7 @@ fail: static void service_enter_stop_by_notify(Service *s) { assert(s); - unit_watch_all_pids(UNIT(s)); + (void) unit_enqueue_rewatch_pids(UNIT(s)); service_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec)); @@ -1789,7 +1795,7 @@ static void service_enter_stop(Service *s, ServiceResult f) { s->result = f; service_unwatch_control_pid(s); - unit_watch_all_pids(UNIT(s)); + (void) unit_enqueue_rewatch_pids(UNIT(s)); s->control_command = s->exec_command[SERVICE_EXEC_STOP]; if (s->control_command) { @@ -3290,11 +3296,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { /* If we get a SIGCHLD event for one of the processes we were interested in, then we look for others to watch, * under the assumption that we'll sooner or later get a SIGCHLD for them, as the original process we watched * was probably the parent of them, and they are hence now our children. */ - unit_tidy_watch_pids(u, s->main_pid, s->control_pid); - unit_watch_all_pids(u); - - /* If the PID set is empty now, then let's check if the cgroup is empty too and finish off the unit. */ - unit_synthesize_cgroup_empty_event(u); + (void) unit_enqueue_rewatch_pids(u); } static int service_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata) { diff --git a/src/core/unit.c b/src/core/unit.c index bfe63365dd7..7265f95c953 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -567,8 +567,9 @@ void unit_free(Unit *u) { unit_done(u); - sd_bus_slot_unref(u->match_bus_slot); + unit_dequeue_rewatch_pids(u); + sd_bus_slot_unref(u->match_bus_slot); sd_bus_track_unref(u->bus_track); u->deserialized_refs = strv_free(u->deserialized_refs); @@ -2605,7 +2606,8 @@ void unit_unwatch_all_pids(Unit *u) { u->pids = set_free(u->pids); } -void unit_tidy_watch_pids(Unit *u, pid_t except1, pid_t except2) { +static void unit_tidy_watch_pids(Unit *u) { + pid_t except1, except2; Iterator i; void *e; @@ -2613,6 +2615,9 @@ void unit_tidy_watch_pids(Unit *u, pid_t except1, pid_t except2) { /* Cleans dead PIDs from our list */ + except1 = unit_main_pid(u); + except2 = unit_control_pid(u); + SET_FOREACH(e, u->pids, i) { pid_t pid = PTR_TO_PID(e); @@ -2624,6 +2629,76 @@ void unit_tidy_watch_pids(Unit *u, pid_t except1, pid_t except2) { } } +static int on_rewatch_pids_event(sd_event_source *s, void *userdata) { + Unit *u = userdata; + + assert(s); + assert(u); + + unit_tidy_watch_pids(u); + unit_watch_all_pids(u); + + /* If the PID set is empty now, then let's finish this off. */ + unit_synthesize_cgroup_empty_event(u); + + return 0; +} + +int unit_enqueue_rewatch_pids(Unit *u) { + int r; + + assert(u); + + if (!u->cgroup_path) + return -ENOENT; + + r = cg_unified_controller(SYSTEMD_CGROUP_CONTROLLER); + if (r < 0) + return r; + if (r > 0) /* On unified we can use proper notifications */ + return 0; + + /* Enqueues a low-priority job that will clean up dead PIDs from our list of PIDs to watch and subscribe to new + * PIDs that might have appeared. We do this in a delayed job because the work might be quite slow, as it + * involves issuing kill(pid, 0) on all processes we watch. */ + + if (!u->rewatch_pids_event_source) { + _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; + + r = sd_event_add_defer(u->manager->event, &s, on_rewatch_pids_event, u); + if (r < 0) + return log_error_errno(r, "Failed to allocate event source for tidying watched PIDs: %m"); + + r = sd_event_source_set_priority(s, SD_EVENT_PRIORITY_IDLE); + if (r < 0) + return log_error_errno(r, "Failed to adjust priority of event source for tidying watched PIDs: m"); + + (void) sd_event_source_set_description(s, "tidy-watch-pids"); + + u->rewatch_pids_event_source = TAKE_PTR(s); + } + + r = sd_event_source_set_enabled(u->rewatch_pids_event_source, SD_EVENT_ONESHOT); + if (r < 0) + return log_error_errno(r, "Failed to enable event source for tidying watched PIDs: %m"); + + return 0; +} + +void unit_dequeue_rewatch_pids(Unit *u) { + int r; + assert(u); + + if (!u->rewatch_pids_event_source) + return; + + r = sd_event_source_set_enabled(u->rewatch_pids_event_source, SD_EVENT_OFF); + if (r < 0) + log_warning_errno(r, "Failed to disable event source for tidying watched PIDs, ignoring: %m"); + + u->rewatch_pids_event_source = sd_event_source_unref(u->rewatch_pids_event_source); +} + bool unit_job_is_applicable(Unit *u, JobType j) { assert(u); assert(j >= 0 && j < _JOB_TYPE_MAX); diff --git a/src/core/unit.h b/src/core/unit.h index f1b5d61af70..76270a0a7c7 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -279,6 +279,10 @@ typedef struct Unit { uint64_t ip_accounting_extra[_CGROUP_IP_ACCOUNTING_METRIC_MAX]; + /* Low-priority event source which is used to remove watched PIDs that have gone away, and subscribe to any new + * ones which might have appeared. */ + sd_event_source *rewatch_pids_event_source; + /* How to start OnFailure units */ JobMode on_failure_job_mode; @@ -650,7 +654,8 @@ int unit_watch_pid(Unit *u, pid_t pid); void unit_unwatch_pid(Unit *u, pid_t pid); void unit_unwatch_all_pids(Unit *u); -void unit_tidy_watch_pids(Unit *u, pid_t except1, pid_t except2); +int unit_enqueue_rewatch_pids(Unit *u); +void unit_dequeue_rewatch_pids(Unit *u); int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name); int unit_watch_bus_name(Unit *u, const char *name);