Merge pull request #26835 from poettering/userdb-modernizations

various userdbd modernizations

3 files changed, 45 insertions, 76 deletions

diff --git a/src/userdb/userdbd-manager.c b/src/userdb/userdbd-manager.c
index 997ef83f57..80735b3fd9 100644
--- a/src/userdb/userdbd-manager.c
+++ b/src/userdb/userdbd-manager.c
@@ -20,49 +20,27 @@
 
 static int start_workers(Manager *m, bool explicit_request);
 
-static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) {
+static int on_worker_exit(sd_event_source *s, const siginfo_t *si, void *userdata) {
         Manager *m = ASSERT_PTR(userdata);
 
         assert(s);
 
-        for (;;) {
-                siginfo_t siginfo = {};
-                bool removed = false;
+        assert_se(!set_remove(m->workers_dynamic, s) != !set_remove(m->workers_fixed, s));
+        sd_event_source_disable_unref(s);
 
-                if (waitid(P_ALL, 0, &siginfo, WNOHANG|WEXITED) < 0) {
-                        if (errno == ECHILD)
-                                break;
-
-                        log_warning_errno(errno, "Failed to invoke waitid(): %m");
-                        break;
-                }
-                if (siginfo.si_pid == 0)
-                        break;
-
-                if (set_remove(m->workers_dynamic, PID_TO_PTR(siginfo.si_pid)))
-                        removed = true;
-                if (set_remove(m->workers_fixed, PID_TO_PTR(siginfo.si_pid)))
-                        removed = true;
-
-                if (!removed) {
-                        log_warning("Weird, got SIGCHLD for unknown child " PID_FMT ", ignoring.", siginfo.si_pid);
-                        continue;
-                }
-
-                if (siginfo.si_code == CLD_EXITED) {
-                        if (siginfo.si_status == EXIT_SUCCESS)
-                                log_debug("Worker " PID_FMT " exited successfully.", siginfo.si_pid);
-                        else
-                                log_warning("Worker " PID_FMT " died with a failure exit status %i, ignoring.", siginfo.si_pid, siginfo.si_status);
-                } else if (siginfo.si_code == CLD_KILLED)
-                        log_warning("Worker " PID_FMT " was killed by signal %s, ignoring.", siginfo.si_pid, signal_to_string(siginfo.si_status));
-                else if (siginfo.si_code == CLD_DUMPED)
-                        log_warning("Worker " PID_FMT " dumped core by signal %s, ignoring.", siginfo.si_pid, signal_to_string(siginfo.si_status));
+        if (si->si_code == CLD_EXITED) {
+                if (si->si_status == EXIT_SUCCESS)
+                        log_debug("Worker " PID_FMT " exited successfully.", si->si_pid);
                 else
-                        log_warning("Can't handle SIGCHLD of this type");
-        }
+                        log_warning("Worker " PID_FMT " died with a failure exit status %i, ignoring.", si->si_pid, si->si_status);
+        } else if (si->si_code == CLD_KILLED)
+                log_warning("Worker " PID_FMT " was killed by signal %s, ignoring.", si->si_pid, signal_to_string(si->si_status));
+        else if (si->si_code == CLD_DUMPED)
+                log_warning("Worker " PID_FMT " dumped core by signal %s, ignoring.", si->si_pid, signal_to_string(si->si_status));
+        else
+                log_warning("Can't handle SIGCHLD of this type");
 
-        (void) start_workers(m, false); /* Fill up workers again if we fell below the low watermark */
+        (void) start_workers(m, /* explicit_request= */ false); /* Fill up workers again if we fell below the low watermark */
         return 0;
 }
 
@@ -75,6 +53,13 @@ static int on_sigusr2(sd_event_source *s, const struct signalfd_siginfo *si, voi
         return 0;
 }
 
+DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(
+                event_source_hash_ops,
+                sd_event_source,
+                (void (*)(const sd_event_source*, struct siphash*)) trivial_hash_func,
+                (int (*)(const sd_event_source*, const sd_event_source*)) trivial_compare_func,
+                sd_event_source_disable_unref);
+
 int manager_new(Manager **ret) {
         _cleanup_(manager_freep) Manager *m = NULL;
         int r;
@@ -95,15 +80,11 @@ int manager_new(Manager **ret) {
         if (r < 0)
                 return r;
 
-        r = sd_event_add_signal(m->event, NULL, SIGINT, NULL, NULL);
+        r = sd_event_set_signal_exit(m->event, true);
         if (r < 0)
                 return r;
 
-        r = sd_event_add_signal(m->event, NULL, SIGTERM, NULL, NULL);
-        if (r < 0)
-                return r;
-
-        r = sd_event_add_signal(m->event, NULL, SIGRTMIN+18, sigrtmin18_handler, NULL);
+        r = sd_event_add_signal(m->event, NULL, (SIGRTMIN+18)|SD_EVENT_SIGNAL_PROCMASK, sigrtmin18_handler, NULL);
         if (r < 0)
                 return r;
 
@@ -111,19 +92,11 @@ int manager_new(Manager **ret) {
         if (r < 0)
                 log_debug_errno(r, "Failed allocate memory pressure event source, ignoring: %m");
 
-        (void) sd_event_set_watchdog(m->event, true);
-
-        m->workers_fixed = set_new(NULL);
-        m->workers_dynamic = set_new(NULL);
-
-        if (!m->workers_fixed || !m->workers_dynamic)
-                return -ENOMEM;
-
-        r = sd_event_add_signal(m->event, &m->sigusr2_event_source, SIGUSR2, on_sigusr2, m);
+        r = sd_event_set_watchdog(m->event, true);
         if (r < 0)
-                return r;
+                log_debug_errno(r, "Failed to enable watchdog handling, ignoring: %m");
 
-        r = sd_event_add_signal(m->event, &m->sigchld_event_source, SIGCHLD, on_sigchld, m);
+        r = sd_event_add_signal(m->event, NULL, SIGUSR2|SD_EVENT_SIGNAL_PROCMASK, on_sigusr2, m);
         if (r < 0)
                 return r;
 
@@ -138,9 +111,6 @@ Manager* manager_free(Manager *m) {
         set_free(m->workers_fixed);
         set_free(m->workers_dynamic);
 
-        sd_event_source_disable_unref(m->sigusr2_event_source);
-        sd_event_source_disable_unref(m->sigchld_event_source);
-
         sd_event_unref(m->event);
 
         return mfree(m);
@@ -153,6 +123,7 @@ static size_t manager_current_workers(Manager *m) {
 }
 
 static int start_one_worker(Manager *m) {
+        _cleanup_(sd_event_source_disable_unrefp) sd_event_source *source = NULL;
         bool fixed;
         pid_t pid;
         int r;
@@ -161,23 +132,18 @@ static int start_one_worker(Manager *m) {
 
         fixed = set_size(m->workers_fixed) < USERDB_WORKERS_MIN;
 
-        r = safe_fork("(sd-worker)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid);
+        r = safe_fork_full(
+                        "(sd-worker)",
+                        /* stdio_fds= */ NULL,
+                        &m->listen_fd, 1,
+                        FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_REOPEN_LOG|FORK_LOG|FORK_CLOSE_ALL_FDS,
+                        &pid);
         if (r < 0)
                 return log_error_errno(r, "Failed to fork new worker child: %m");
         if (r == 0) {
                 char pids[DECIMAL_STR_MAX(pid_t)];
                 /* Child */
 
-                log_close();
-
-                r = close_all_fds(&m->listen_fd, 1);
-                if (r < 0) {
-                        log_error_errno(r, "Failed to close fds in child: %m");
-                        _exit(EXIT_FAILURE);
-                }
-
-                log_open();
-
                 if (m->listen_fd == 3) {
                         r = fd_cloexec(3, false);
                         if (r < 0) {
@@ -218,13 +184,19 @@ static int start_one_worker(Manager *m) {
                 _exit(EXIT_FAILURE);
         }
 
-        if (fixed)
-                r = set_put(m->workers_fixed, PID_TO_PTR(pid));
-        else
-                r = set_put(m->workers_dynamic, PID_TO_PTR(pid));
+        r = sd_event_add_child(m->event, &source, pid, WEXITED, on_worker_exit, m);
+        if (r < 0)
+                return log_error_errno(r, "Failed to watch child " PID_FMT ": %m", pid);
+
+        r = set_ensure_put(
+                        fixed ? &m->workers_fixed : &m->workers_dynamic,
+                        &event_source_hash_ops,
+                        source);
         if (r < 0)
                 return log_error_errno(r, "Failed to add child process to set: %m");
 
+        TAKE_PTR(source);
+
         return 0;
 }
 
@@ -271,7 +243,7 @@ int manager_startup(Manager *m) {
         if (n == 1)
                 m->listen_fd = SD_LISTEN_FDS_START;
         else {
-                union sockaddr_union sockaddr = {
+                static const union sockaddr_union sockaddr = {
                         .un.sun_family = AF_UNIX,
                         .un.sun_path = "/run/systemd/userdb/io.systemd.Multiplexer",
                 };
diff --git a/src/userdb/userdbd-manager.h b/src/userdb/userdbd-manager.h
index 35931da4c0..4fb45e6c35 100644
--- a/src/userdb/userdbd-manager.h
+++ b/src/userdb/userdbd-manager.h
@@ -18,9 +18,6 @@ struct Manager {
         Set *workers_fixed;    /* Workers 0…USERDB_WORKERS_MIN */
         Set *workers_dynamic;  /* Workers USERD_WORKERS_MIN+1…USERDB_WORKERS_MAX */
 
-        sd_event_source *sigusr2_event_source;
-        sd_event_source *sigchld_event_source;
-
         int listen_fd;
 
         RateLimit worker_ratelimit;
diff --git a/src/userdb/userdbd.c b/src/userdb/userdbd.c
index 2bf37a52c4..89ac9c7ba9 100644
--- a/src/userdb/userdbd.c
+++ b/src/userdb/userdbd.c
@@ -37,7 +37,7 @@ static int run(int argc, char *argv[]) {
         if (setenv("SYSTEMD_BYPASS_USERDB", "io.systemd.NameServiceSwitch:io.systemd.Multiplexer:io.systemd.DropIn", 1) < 0)
                 return log_error_errno(errno, "Failed to set $SYSTEMD_BYPASS_USERDB: %m");
 
-        assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, SIGTERM, SIGINT, SIGUSR2, SIGRTMIN+18, -1) >= 0);
+        assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, -1) >= 0);
 
         r = manager_new(&m);
         if (r < 0)