Merge c6679e0bd4 into 94eacb9329

Even if there are long running spawned processes.

src/core/device.c

@@ -1048,9 +1048,6 @@ static void device_enumerate(Manager *m) {
                 _cleanup_set_free_ Set *ready_units = NULL, *not_ready_units = NULL;
                 Device *d;
 
-                if (device_is_processed(dev) <= 0)
-                        continue;
-
                 if (device_setup_units(m, dev, &ready_units, &not_ready_units) < 0)
                         continue;
 

src/udev/test-udev-spawn.c

@@ -10,17 +10,44 @@
 
 #define BUF_SIZE 1024
 
-static void test_event_spawn_core(bool with_pidfd, const char *cmd, char *result_buf, size_t buf_size) {
+static void test_event_spawn_core(bool with_pidfd, const char *cmd, char *result_buf, size_t buf_size, int expected) {
         _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
         _cleanup_(udev_event_freep) UdevEvent *event = NULL;
 
-        assert_se(setenv("SYSTEMD_PIDFD", yes_no(with_pidfd), 1) >= 0);
+        ASSERT_OK_ERRNO(setenv("SYSTEMD_PIDFD", yes_no(with_pidfd), 1));
 
-        assert_se(sd_device_new_from_syspath(&dev, "/sys/class/net/lo") >= 0);
+        ASSERT_OK(sd_device_new_from_syspath(&dev, "/sys/class/net/lo"));
-        assert_se(event = udev_event_new(dev, NULL, EVENT_TEST_SPAWN));
+        ASSERT_NOT_NULL(event = udev_event_new(dev, NULL, EVENT_TEST_SPAWN));
-        assert_se(udev_event_spawn(event, false, cmd, result_buf, buf_size, NULL) == 0);
+        ASSERT_EQ(udev_event_spawn(event, false, cmd, result_buf, buf_size, NULL), expected);
 
-        assert_se(unsetenv("SYSTEMD_PIDFD") >= 0);
+        ASSERT_OK_ERRNO(unsetenv("SYSTEMD_PIDFD"));
+}
+
+static void test_event_spawn_sleep_child(bool with_pidfd) {
+        _cleanup_free_ char *cmd = NULL;
+
+        log_debug("/* %s(%s) */", __func__, yes_no(with_pidfd));
+
+        ASSERT_OK(find_executable("sleep", &cmd));
+        ASSERT_NOT_NULL(strextend_with_separator(&cmd, " ", "1h"));
+
+        test_event_spawn_core(with_pidfd, cmd, NULL, 0, -EIO);
+}
+
+static void test_event_spawn_sleep(bool with_pidfd) {
+        PidRef pidref;
+        int r;
+
+        r = pidref_safe_fork("(test-worker)", FORK_LOG, &pidref);
+        ASSERT_OK(r);
+        if (r == 0) {
+                test_event_spawn_sleep_child(with_pidfd);
+                _exit(EXIT_SUCCESS);
+        }
+
+        ASSERT_OK(usleep_safe(USEC_PER_SEC));
+        ASSERT_OK(pidref_kill(&pidref, SIGTERM));
+        ASSERT_OK(wait_for_terminate_with_timeout(pidref.pid, 10 * USEC_PER_SEC));
 }
 
 static void test_event_spawn_cat(bool with_pidfd, size_t buf_size) {
@@ -30,18 +57,18 @@ static void test_event_spawn_cat(bool with_pidfd, size_t buf_size) {
 
         log_debug("/* %s(%s) */", __func__, yes_no(with_pidfd));
 
-        assert_se(find_executable("cat", &cmd) >= 0);
+        ASSERT_OK(find_executable("cat", &cmd));
-        assert_se(strextend_with_separator(&cmd, " ", "/sys/class/net/lo/uevent"));
+        ASSERT_NOT_NULL(strextend_with_separator(&cmd, " ", "/sys/class/net/lo/uevent"));
 
         test_event_spawn_core(with_pidfd, cmd, result_buf,
-                              buf_size >= BUF_SIZE ? BUF_SIZE : buf_size);
+                              buf_size >= BUF_SIZE ? BUF_SIZE : buf_size, 0);
 
-        assert_se(lines = strv_split_newlines(result_buf));
+        ASSERT_NOT_NULL(lines = strv_split_newlines(result_buf));
         strv_print(lines);
 
         if (buf_size >= BUF_SIZE) {
-                assert_se(strv_contains(lines, "INTERFACE=lo"));
+                ASSERT_TRUE(strv_contains(lines, "INTERFACE=lo"));
-                assert_se(strv_contains(lines, "IFINDEX=1"));
+                ASSERT_TRUE(strv_contains(lines, "IFINDEX=1"));
         }
 }
 
@@ -53,15 +80,15 @@ static void test_event_spawn_self(const char *self, const char *arg, bool with_p
         log_debug("/* %s(%s, %s) */", __func__, arg, yes_no(with_pidfd));
 
         /* 'self' may contain spaces, hence needs to be quoted. */
-        assert_se(cmd = strjoin("'", self, "' ", arg));
+        ASSERT_NOT_NULL(cmd = strjoin("'", self, "' ", arg));
 
-        test_event_spawn_core(with_pidfd, cmd, result_buf, BUF_SIZE);
+        test_event_spawn_core(with_pidfd, cmd, result_buf, BUF_SIZE, 0);
 
-        assert_se(lines = strv_split_newlines(result_buf));
+        ASSERT_NOT_NULL(lines = strv_split_newlines(result_buf));
         strv_print(lines);
 
-        assert_se(strv_contains(lines, "aaa"));
+        ASSERT_TRUE(strv_contains(lines, "aaa"));
-        assert_se(strv_contains(lines, "bbb"));
+        ASSERT_TRUE(strv_contains(lines, "bbb"));
 }
 
 static void test1(void) {
@@ -114,5 +141,7 @@ int main(int argc, char *argv[]) {
         test_event_spawn_self(self, "test2", true);
         test_event_spawn_self(self, "test2", false);
 
+        test_event_spawn_sleep(true);
+        test_event_spawn_sleep(false);
         return 0;
 }

src/udev/udev-spawn.c

@@ -16,6 +16,7 @@
 #include "udev-trace.h"
 
 typedef struct Spawn {
+        UdevWorker *worker;
         sd_device *device;
         const char *cmd;
         pid_t pid;
@@ -151,11 +152,27 @@ static int on_spawn_sigchld(sd_event_source *s, const siginfo_t *si, void *userd
         return 1;
 }
 
+static int on_spawn_sigterm(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) {
+        Spawn *spawn = ASSERT_PTR(userdata);
+
+        DEVICE_TRACE_POINT(spawn_sigterm, spawn->device, spawn->cmd);
+
+        log_device_error(spawn->device, "Worker process received SIGTERM, killing spawned process '%s' ["PID_FMT"].",
+                         spawn->cmd, spawn->pid);
+
+        (void) kill_and_sigcont(spawn->pid, SIGTERM);
+
+        /* terminate the main event source of the worker. Note, the spawn->worker may be NULL in tests. */
+        if (spawn->worker)
+                (void) sd_event_exit(spawn->worker->event, 0);
+
+        return 1;
+}
+
 static int spawn_wait(Spawn *spawn) {
         _cleanup_(sd_event_unrefp) sd_event *e = NULL;
-        _cleanup_(sd_event_source_disable_unrefp) sd_event_source *sigchld_source = NULL;
+        _cleanup_(sd_event_source_disable_unrefp) sd_event_source
-        _cleanup_(sd_event_source_disable_unrefp) sd_event_source *stdout_source = NULL;
+                *sigchld_source = NULL, *stdout_source = NULL, *stderr_source = NULL, *sigterm_source = NULL;
-        _cleanup_(sd_event_source_disable_unrefp) sd_event_source *stderr_source = NULL;
         int r;
 
         assert(spawn);
@@ -201,11 +218,21 @@ static int spawn_wait(Spawn *spawn) {
         r = sd_event_add_child(e, &sigchld_source, spawn->pid, WEXITED, on_spawn_sigchld, spawn);
         if (r < 0)
                 return log_device_debug_errno(spawn->device, r, "Failed to create sigchild event source: %m");
+
         /* SIGCHLD should be processed after IO is complete */
         r = sd_event_source_set_priority(sigchld_source, SD_EVENT_PRIORITY_NORMAL + 1);
         if (r < 0)
                 return log_device_debug_errno(spawn->device, r, "Failed to set priority to sigchild event source: %m");
 
+        r = sd_event_add_signal(e, &sigterm_source, SIGTERM | SD_EVENT_SIGNAL_PROCMASK, on_spawn_sigterm, spawn);
+        if (r < 0)
+                return log_device_debug_errno(spawn->device, r, "Failed to set SIGTERM event: %m");
+
+        /* SIGTERM should be processed with higher priorities with the others. */
+        r = sd_event_source_set_priority(sigterm_source, SD_EVENT_PRIORITY_NORMAL - 1);
+        if (r < 0)
+                return log_device_debug_errno(spawn->device, r, "Failed to set priority to sigchild event source: %m");
+
         return sd_event_loop(e);
 }
 
@@ -239,6 +266,10 @@ int udev_event_spawn(
                 return 0;
         }
 
+        if (event->worker && sd_event_get_exit_code(event->worker->event, NULL) >= 0)
+                return log_device_debug_errno(event->dev, SYNTHETIC_ERRNO(EIO),
+                                              "The main event loop of the worker process already terminating, skipping execution of '%s'.", cmd);
+
         int timeout_signal = event->worker ? event->worker->timeout_signal : SIGKILL;
         usec_t timeout_usec = event->worker ? event->worker->timeout_usec : DEFAULT_WORKER_TIMEOUT_USEC;
         usec_t now_usec = now(CLOCK_MONOTONIC);
@@ -304,6 +335,7 @@ int udev_event_spawn(
         errpipe[WRITE_END] = safe_close(errpipe[WRITE_END]);
 
         spawn = (Spawn) {
+                .worker = event->worker,
                 .device = event->dev,
                 .cmd = cmd,
                 .pid = pid,

test/units/TEST-17-UDEV.device_is_processing.sh

@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: LGPL-2.1-or-later
+set -ex
+set -o pipefail
+
+# This is a reproducer of issue #35329,
+# which is a regression caused by 405be62f05d76f1845f347737b5972158c79dd3e.
+
+IFNAME=udevtestnetif
+
+udevadm settle
+
+mkdir -p /run/udev/udev.conf.d/
+cat >/run/udev/udev.conf.d/timeout.conf <<EOF
+event_timeout=1h
+EOF
+
+mkdir -p /run/udev/rules.d/
+cat >/run/udev/rules.d/99-testsuite.rules <<EOF
+SUBSYSTEM=="net", ACTION=="change", KERNEL=="${IFNAME}", OPTIONS="log_level=debug", RUN+="/usr/bin/sleep 1000"
+EOF
+
+systemctl restart systemd-udevd.service
+
+ip link add "$IFNAME" type dummy
+IFINDEX=$(ip -json link show "$IFNAME" | jq '.[].ifindex')
+udevadm wait --timeout 10 "/sys/class/net/${IFNAME}"
+# Check if the database file is created.
+[[ -e "/run/udev/data/n${IFINDEX}" ]]
+
+systemd-run \
+    -p After="sys-subsystem-net-devices-${IFNAME}.device" \
+    -p BindsTo="sys-subsystem-net-devices-${IFNAME}.device" \
+    -u testsleep.service \
+    sleep 1h
+
+timeout 10 bash -c 'until systemctl is-active testsleep.service; do sleep .5; done'
+
+udevadm trigger "/sys/class/net/${IFNAME}"
+timeout 30 bash -c "until grep -F 'ID_PROCESSING=1' /run/udev/data/n${IFINDEX}; do sleep .5; done"
+
+for _ in {1..3}; do
+    systemctl daemon-reexec
+    systemctl is-active testsleep.service
+done
+
+for _ in {1..3}; do
+    systemctl daemon-reload
+    systemctl is-active testsleep.service
+done
+
+# Check if the reexec and reload have finished during processing the event.
+grep -F 'ID_PROCESSING=1' "/run/udev/data/n${IFINDEX}"
+
+# cleanup
+systemctl stop testsleep.service
+rm -f /run/udev/udev.conf.d/timeout.conf
+rm -f /run/udev/rules.d/99-testsuite.rules
+# Check if udevd can be restarted within a reasonably short time.
+timeout 10 systemctl restart systemd-udevd.service
+ip link del "$IFNAME"
+
+exit 0