udevd: event - port spawn_wait() to sd-event

This allows us to drop the special sigterm handling in spawn_wait()
as this will now be passed directly to the worker event loop.

We now log failing spawend processes at 'warning' level, and timeouts
are in terms of CLOCK_BOOTTIME when available, otherwise the behavior
is unchanged.

diff --git a/src/test/test-udev.c b/src/test/test-udev.c
index 23b7faa93923089b4742a4b5870432bfc96ebbcd..f3953fe26a745aea6f1b59f587eb92136b2d1885 100644 (file)
--- a/src/test/test-udev.c
+++ b/src/test/test-udev.c
@@ -120,11 +120,6 @@ int main(int argc, char *argv[]) {
 
         sigfillset(&mask);
         sigprocmask(SIG_SETMASK, &mask, &sigmask_orig);
-        event->fd_signal = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
-        if (event->fd_signal < 0) {
-                fprintf(stderr, "error creating signalfd\n");
-                goto out;
-        }
 
         /* do what devtmpfs usually provides us */
         if (udev_device_get_devnode(dev) != NULL) {
@@ -153,8 +148,6 @@ int main(int argc, char *argv[]) {
                                3 * USEC_PER_SEC, USEC_PER_SEC,
                                NULL);
 out:
-        if (event != NULL && event->fd_signal >= 0)
-                close(event->fd_signal);
         mac_selinux_finish();
 
         return err ? EXIT_FAILURE : EXIT_SUCCESS;

diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index 2fa26a40be182b2e7d7c683c7c733709ec3cadb6..92dc44f68794a3e98273d24a64a3a6914854a178 100644 (file)
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -32,7 +32,16 @@
 
 #include "udev.h"
 #include "rtnl-util.h"
+#include "event-util.h"
 #include "formats-util.h"
+#include "process-util.h"
+
+typedef struct Spawn {
+        const char *cmd;
+        pid_t pid;
+        usec_t timeout_warn;
+        usec_t timeout;
+} Spawn;
 
 struct udev_event *udev_event_new(struct udev_device *dev) {
         struct udev *udev = udev_device_get_udev(dev);
@@ -45,8 +54,7 @@ struct udev_event *udev_event_new(struct udev_device *dev) {
         event->udev = udev;
         udev_list_init(udev, &event->run_list, false);
         udev_list_init(udev, &event->seclabel_list, false);
-        event->fd_signal = -1;
-        event->birth_usec = now(CLOCK_MONOTONIC);
+        event->birth_usec = clock_boottime_or_monotonic();
         return event;
 }
 
@@ -467,7 +475,7 @@ static void spawn_read(struct udev_event *event,
                 if (timeout_usec > 0) {
                         usec_t age_usec;
 
-                        age_usec = now(CLOCK_MONOTONIC) - event->birth_usec;
+                        age_usec = clock_boottime_or_monotonic() - event->birth_usec;
                         if (age_usec >= timeout_usec) {
                                 log_error("timeout '%s'", cmd);
                                 return;
@@ -540,102 +548,116 @@ static void spawn_read(struct udev_event *event,
                 result[respos] = '\0';
 }
 
+static int on_spawn_timeout(sd_event_source *s, uint64_t usec, void *userdata) {
+        Spawn *spawn = userdata;
+        char timeout[FORMAT_TIMESTAMP_RELATIVE_MAX];
+
+        assert(spawn);
+
+        kill_and_sigcont(spawn->pid, SIGKILL);
+
+        log_error("spawned process '%s' ["PID_FMT"] timed out after %s, killing", spawn->cmd, spawn->pid,
+                  format_timestamp_relative(timeout, sizeof(timeout), spawn->timeout));
+
+        return 1;
+}
+
+static int on_spawn_timeout_warning(sd_event_source *s, uint64_t usec, void *userdata) {
+        Spawn *spawn = userdata;
+        char timeout[FORMAT_TIMESTAMP_RELATIVE_MAX];
+
+        assert(spawn);
+
+        log_warning("spawned process '%s' ["PID_FMT"] is taking longer than %s to complete", spawn->cmd, spawn->pid,
+                    format_timestamp_relative(timeout, sizeof(timeout), spawn->timeout));
+
+        return 1;
+}
+
+static int on_spawn_sigchld(sd_event_source *s, const siginfo_t *si, void *userdata) {
+        Spawn *spawn = userdata;
+
+        assert(spawn);
+
+        switch (si->si_code) {
+        case CLD_EXITED:
+                if (si->si_status != 0)
+                        log_warning("process '%s' failed with exit code %i.", spawn->cmd, si->si_status);
+                else {
+                        log_debug("process '%s' succeeded.", spawn->cmd);
+                        sd_event_exit(sd_event_source_get_event(s), 0);
+
+                        return 1;
+                }
+
+                break;
+        case CLD_KILLED:
+        case CLD_DUMPED:
+                log_warning("process '%s' terminated by signal %s.", spawn->cmd, signal_to_string(si->si_status));
+
+                break;
+        default:
+                log_error("process '%s' failed due to unknown reason.", spawn->cmd);
+        }
+
+        sd_event_exit(sd_event_source_get_event(s), -EIO);
+
+        return 1;
+}
+
 static int spawn_wait(struct udev_event *event,
                       usec_t timeout_usec,
                       usec_t timeout_warn_usec,
                       const char *cmd, pid_t pid) {
-        struct pollfd pfd[1];
-        int err = 0;
-
-        pfd[0].events = POLLIN;
-        pfd[0].fd = event->fd_signal;
+        Spawn spawn = {
+                .cmd = cmd,
+                .pid = pid,
+        };
+        _cleanup_event_unref_ sd_event *e = NULL;
+        int r, ret;
 
-        while (pid > 0) {
-                int timeout;
-                int timeout_warn = 0;
-                int fdcount;
+        r = sd_event_new(&e);
+        if (r < 0)
+                return r;
 
-                if (timeout_usec > 0) {
-                        usec_t age_usec;
+        if (timeout_usec > 0) {
+                usec_t usec, age_usec;
 
-                        age_usec = now(CLOCK_MONOTONIC) - event->birth_usec;
-                        if (age_usec >= timeout_usec)
-                                timeout = 1000;
-                        else {
-                                if (timeout_warn_usec > 0)
-                                        timeout_warn = ((timeout_warn_usec - age_usec) / USEC_PER_MSEC) + MSEC_PER_SEC;
+                usec = now(clock_boottime_or_monotonic());
+                age_usec = usec - event->birth_usec;
+                if (age_usec < timeout_usec) {
+                        if (timeout_warn_usec > 0 && timeout_warn_usec < timeout_usec && age_usec < timeout_warn_usec) {
+                                spawn.timeout_warn = timeout_warn_usec - age_usec;
 
-                                timeout = ((timeout_usec - timeout_warn_usec - age_usec) / USEC_PER_MSEC) + MSEC_PER_SEC;
+                                r =  sd_event_add_time(e, NULL, clock_boottime_or_monotonic(),
+                                                       usec + spawn.timeout_warn, USEC_PER_SEC,
+                                                       on_spawn_timeout_warning, &spawn);
+                                if (r < 0)
+                                        return r;
                         }
-                } else {
-                        timeout = -1;
-                }
 
-                fdcount = poll(pfd, 1, timeout_warn);
-                if (fdcount < 0) {
-                        if (errno == EINTR)
-                                continue;
-                        err = -errno;
-                        log_error_errno(errno, "failed to poll: %m");
-                        goto out;
-                }
-                if (fdcount == 0) {
-                        log_warning("slow: '%s' ["PID_FMT"]", cmd, pid);
+                        spawn.timeout = timeout_usec - age_usec;
 
-                        fdcount = poll(pfd, 1, timeout);
-                        if (fdcount < 0) {
-                                if (errno == EINTR)
-                                        continue;
-                                err = -errno;
-                                log_error_errno(errno, "failed to poll: %m");
-                                goto out;
-                        }
-                        if (fdcount == 0) {
-                                log_error("timeout: killing '%s' ["PID_FMT"]", cmd, pid);
-                                kill(pid, SIGKILL);
-                        }
+                        r = sd_event_add_time(e, NULL, clock_boottime_or_monotonic(),
+                                              usec + spawn.timeout, USEC_PER_SEC, on_spawn_timeout, &spawn);
+                        if (r < 0)
+                                return r;
                 }
+        }
 
-                if (pfd[0].revents & POLLIN) {
-                        struct signalfd_siginfo fdsi;
-                        int status;
-                        ssize_t size;
+        r = sd_event_add_child(e, NULL, pid, WEXITED, on_spawn_sigchld, &spawn);
+        if (r < 0)
+                return r;
 
-                        size = read(event->fd_signal, &fdsi, sizeof(struct signalfd_siginfo));
-                        if (size != sizeof(struct signalfd_siginfo))
-                                continue;
+        r = sd_event_loop(e);
+        if (r < 0)
+                return r;
 
-                        switch (fdsi.ssi_signo) {
-                        case SIGTERM:
-                                event->sigterm = true;
-                                break;
-                        case SIGCHLD:
-                                if (waitpid(pid, &status, WNOHANG) < 0)
-                                        break;
-                                if (WIFEXITED(status)) {
-                                        log_debug("'%s' ["PID_FMT"] exit with return code %i", cmd, pid, WEXITSTATUS(status));
-                                        if (WEXITSTATUS(status) != 0)
-                                                err = -1;
-                                } else if (WIFSIGNALED(status)) {
-                                        log_error("'%s' ["PID_FMT"] terminated by signal %i (%s)", cmd, pid, WTERMSIG(status), strsignal(WTERMSIG(status)));
-                                        err = -1;
-                                } else if (WIFSTOPPED(status)) {
-                                        log_error("'%s' ["PID_FMT"] stopped", cmd, pid);
-                                        err = -1;
-                                } else if (WIFCONTINUED(status)) {
-                                        log_error("'%s' ["PID_FMT"] continued", cmd, pid);
-                                        err = -1;
-                                } else {
-                                        log_error("'%s' ["PID_FMT"] exit with status 0x%04x", cmd, pid, status);
-                                        err = -1;
-                                }
-                                pid = 0;
-                                break;
-                        }
-                }
-        }
-out:
-        return err;
+        r = sd_event_get_exit_code(e, &ret);
+        if (r < 0)
+                return r;
+
+        return ret;
 }
 
 int udev_build_argv(struct udev *udev, char *cmd, int *argc, char *argv[]) {

diff --git a/src/udev/udev.h b/src/udev/udev.h
index dece6eccab22c1ff73ed78ff85e9f11c8934e8e9..1b17c615b89d813bbcc5b557a21dbd284c61f9b7 100644 (file)
--- a/src/udev/udev.h
+++ b/src/udev/udev.h
@@ -44,11 +44,9 @@ struct udev_event {
         struct udev_list run_list;
         int exec_delay;
         usec_t birth_usec;
-        int fd_signal;
         sd_rtnl *rtnl;
         unsigned int builtin_run;
         unsigned int builtin_ret;
-        bool sigterm;
         bool inotify_watch;
         bool inotify_watch_final;
         bool group_set;

diff --git a/src/udev/udevadm-test.c b/src/udev/udevadm-test.c
index fe092cfbd9af4bc33d580f6bd6b76240a56178cd..46ec0e322511b2a2cd767ed86044ba8a05f42ee9 100644 (file)
--- a/src/udev/udevadm-test.c
+++ b/src/udev/udevadm-test.c
@@ -131,12 +131,6 @@ static int adm_test(struct udev *udev, int argc, char *argv[]) {
 
         sigfillset(&mask);
         sigprocmask(SIG_SETMASK, &mask, &sigmask_orig);
-        event->fd_signal = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
-        if (event->fd_signal < 0) {
-                fprintf(stderr, "error creating signalfd\n");
-                rc = 5;
-                goto out;
-        }
 
         udev_event_execute_rules(event,
                                  60 * USEC_PER_SEC, 20 * USEC_PER_SEC,
@@ -154,8 +148,6 @@ static int adm_test(struct udev *udev, int argc, char *argv[]) {
                 printf("run: '%s'\n", program);
         }
 out:
-        if (event != NULL && event->fd_signal >= 0)
-                close(event->fd_signal);
         udev_builtin_exit(udev);
         return rc;
 }

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 3022cd225fad533843569f6cf74c267119b012e2..dbfe5f940fa8c37d5600a5f376e8fa67b57ae56f 100644 (file)
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -406,9 +406,6 @@ static void worker_spawn(Manager *manager, struct event *event) {
                                 goto out;
                         }
 
-                        /* needed for SIGCHLD/SIGTERM in spawn() */
-                        udev_event->fd_signal = fd_signal;
-
                         if (arg_exec_delay > 0)
                                 udev_event->exec_delay = arg_exec_delay;
 
@@ -481,11 +478,6 @@ skip:
                         udev_device_unref(dev);
                         dev = NULL;
 
-                        if (udev_event->sigterm) {
-                                udev_event_unref(udev_event);
-                                goto out;
-                        }
-
                         udev_event_unref(udev_event);
 
                         /* wait for more device messages from main udevd, or term signal */