Merge pull request #19811 from anitazha/revert_mount_rl

sd-event: fix failure to exit rate limiting state

src/core/execute.c

@@ -6371,38 +6371,24 @@ int exec_runtime_deserialize_compat(Unit *u, const char *key, const char *value,
                 return 0;
         }
 
-        r = hashmap_ensure_allocated(&u->manager->exec_runtime_by_id, &string_hash_ops);
+        if (hashmap_ensure_allocated(&u->manager->exec_runtime_by_id, &string_hash_ops) < 0)
-        if (r < 0) {
+                return log_oom();
-                log_unit_debug_errno(u, r, "Failed to allocate storage for runtime parameter: %m");
-                return 0;
-        }
 
         rt = hashmap_get(u->manager->exec_runtime_by_id, u->id);
         if (!rt) {
-                r = exec_runtime_allocate(&rt_create, u->id);
+                if (exec_runtime_allocate(&rt_create, u->id) < 0)
-                if (r < 0)
                         return log_oom();
 
                 rt = rt_create;
         }
 
         if (streq(key, "tmp-dir")) {
-                char *copy;
+                if (free_and_strdup_warn(&rt->tmp_dir, value) < 0)
-
+                        return -ENOMEM;
-                copy = strdup(value);
-                if (!copy)
-                        return log_oom();
-
-                free_and_replace(rt->tmp_dir, copy);
 
         } else if (streq(key, "var-tmp-dir")) {
-                char *copy;
+                if (free_and_strdup_warn(&rt->var_tmp_dir, value) < 0)
-
+                        return -ENOMEM;
-                copy = strdup(value);
-                if (!copy)
-                        return log_oom();
-
-                free_and_replace(rt->var_tmp_dir, copy);
 
         } else if (streq(key, "netns-socket-0")) {
                 int fd;
@@ -6426,27 +6412,6 @@ int exec_runtime_deserialize_compat(Unit *u, const char *key, const char *value,
                 safe_close(rt->netns_storage_socket[1]);
                 rt->netns_storage_socket[1] = fdset_remove(fds, fd);
 
-        } else if (streq(key, "ipcns-socket-0")) {
-                int fd;
-
-                if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd)) {
-                        log_unit_debug(u, "Failed to parse ipcns socket value: %s", value);
-                        return 0;
-                }
-
-                safe_close(rt->ipcns_storage_socket[0]);
-                rt->ipcns_storage_socket[0] = fdset_remove(fds, fd);
-
-        } else if (streq(key, "ipcns-socket-1")) {
-                int fd;
-
-                if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd)) {
-                        log_unit_debug(u, "Failed to parse ipcns socket value: %s", value);
-                        return 0;
-                }
-
-                safe_close(rt->ipcns_storage_socket[1]);
-                rt->ipcns_storage_socket[1] = fdset_remove(fds, fd);
         } else
                 return 0;
 

src/core/unit-serialize.c

@@ -108,7 +108,9 @@ int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool switching_root) {
         fputs(u->id, f);
         fputc('\n', f);
 
-        if (unit_can_serialize(u)) {
+        assert(!!UNIT_VTABLE(u)->serialize == !!UNIT_VTABLE(u)->deserialize_item);
+
+        if (UNIT_VTABLE(u)->serialize) {
                 r = UNIT_VTABLE(u)->serialize(u, f, fds);
                 if (r < 0)
                         return r;
@@ -498,17 +500,15 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) {
                         continue;
                 }
 
-                if (unit_can_serialize(u)) {
+                r = exec_runtime_deserialize_compat(u, l, v, fds);
-                        r = exec_runtime_deserialize_compat(u, l, v, fds);
+                if (r < 0) {
-                        if (r < 0) {
+                        log_unit_warning(u, "Failed to deserialize runtime parameter '%s', ignoring.", l);
-                                log_unit_warning(u, "Failed to deserialize runtime parameter '%s', ignoring.", l);
+                        continue;
-                                continue;
+                } else if (r > 0)
-                        }
-
                         /* Returns positive if key was handled by the call */
-                        if (r > 0)
+                        continue;
-                                continue;
 
+                if (UNIT_VTABLE(u)->deserialize_item) {
                         r = UNIT_VTABLE(u)->deserialize_item(u, l, v, fds);
                         if (r < 0)
                                 log_unit_warning(u, "Failed to deserialize unit parameter '%s', ignoring.", l);

src/core/unit.c

@@ -3488,12 +3488,6 @@ void unit_unwatch_bus_name(Unit *u, const char *name) {
         u->get_name_owner_slot = sd_bus_slot_unref(u->get_name_owner_slot);
 }
 
-bool unit_can_serialize(Unit *u) {
-        assert(u);
-
-        return UNIT_VTABLE(u)->serialize && UNIT_VTABLE(u)->deserialize_item;
-}
-
 int unit_add_node_dependency(Unit *u, const char *what, UnitDependency dep, UnitDependencyMask mask) {
         _cleanup_free_ char *e = NULL;
         Unit *device;

src/core/unit.h

@@ -814,8 +814,6 @@ char *unit_dbus_path_invocation_id(Unit *u);
 
 int unit_load_related_unit(Unit *u, const char *type, Unit **_found);
 
-bool unit_can_serialize(Unit *u) _pure_;
-
 int unit_add_node_dependency(Unit *u, const char *what, UnitDependency d, UnitDependencyMask mask);
 int unit_add_blockdev_dependency(Unit *u, const char *what, UnitDependencyMask mask);
 

src/libsystemd/sd-event/sd-event.c

@@ -237,25 +237,6 @@ static usec_t time_event_source_next(const sd_event_source *s) {
         return USEC_INFINITY;
 }
 
-static int earliest_time_prioq_compare(const void *a, const void *b) {
-        const sd_event_source *x = a, *y = b;
-
-        /* Enabled ones first */
-        if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF)
-                return -1;
-        if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)
-                return 1;
-
-        /* Move the pending ones to the end */
-        if (!x->pending && y->pending)
-                return -1;
-        if (x->pending && !y->pending)
-                return 1;
-
-        /* Order by time */
-        return CMP(time_event_source_next(x), time_event_source_next(y));
-}
-
 static usec_t time_event_source_latest(const sd_event_source *s) {
         assert(s);
 
@@ -274,7 +255,15 @@ static usec_t time_event_source_latest(const sd_event_source *s) {
         return USEC_INFINITY;
 }
 
-static int latest_time_prioq_compare(const void *a, const void *b) {
+static bool event_source_timer_candidate(const sd_event_source *s) {
+        assert(s);
+
+        /* Returns true for event sources that either are not pending yet (i.e. where it's worth to mark them pending)
+         * or which are currently ratelimited (i.e. where it's worth leaving the ratelimited state) */
+        return !s->pending || s->ratelimited;
+}
+
+static int time_prioq_compare(const void *a, const void *b, usec_t (*time_func)(const sd_event_source *s)) {
         const sd_event_source *x = a, *y = b;
 
         /* Enabled ones first */
@@ -283,14 +272,22 @@ static int latest_time_prioq_compare(const void *a, const void *b) {
         if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)
                 return 1;
 
-        /* Move the pending ones to the end */
+        /* Order "non-pending OR ratelimited" before "pending AND not-ratelimited" */
-        if (!x->pending && y->pending)
+        if (event_source_timer_candidate(x) && !event_source_timer_candidate(y))
                 return -1;
-        if (x->pending && !y->pending)
+        if (!event_source_timer_candidate(x) && event_source_timer_candidate(y))
                 return 1;
 
         /* Order by time */
-        return CMP(time_event_source_latest(x), time_event_source_latest(y));
+        return CMP(time_func(x), time_func(y));
+}
+
+static int earliest_time_prioq_compare(const void *a, const void *b) {
+        return time_prioq_compare(a, b, time_event_source_next);
+}
+
+static int latest_time_prioq_compare(const void *a, const void *b) {
+        return time_prioq_compare(a, b, time_event_source_latest);
 }
 
 static int exit_prioq_compare(const void *a, const void *b) {

src/locale/kbd-model-map

@@ -67,3 +67,5 @@ lt.l4			lt	pc105		-		terminate:ctrl_alt_bksp
 lt			lt	pc105		-		terminate:ctrl_alt_bksp
 khmer			kh,us	pc105		-		terminate:ctrl_alt_bksp
 es-dvorak		es	microsoftpro	dvorak		terminate:ctrl_alt_bksp
+lv			lv	pc105		apostrophe	terminate:ctrl_alt_bksp
+lv-tilde		lv	pc105		tilde		terminate:ctrl_alt_bksp

test/TEST-60-MOUNT-RATELIMIT/Makefile

@@ -0,0 +1 @@
+../TEST-01-BASIC/Makefile

test/TEST-60-MOUNT-RATELIMIT/test.sh

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash
+set -e
+TEST_DESCRIPTION="Test that mount/unmount storms can enter/exit rate limit state and will not leak units"
+
+. $TEST_BASE_DIR/test-functions
+
+do_test "$@"

test/units/testsuite-60.service

@@ -0,0 +1,6 @@
+[Unit]
+Description=TEST-60-MOUNT-RATELIMIT
+
+[Service]
+Type=oneshot
+ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh

test/units/testsuite-60.sh

@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+set -eux
+set -o pipefail
+
+systemd-analyze log-level debug
+systemd-analyze log-target journal
+
+NUM_DIRS=20
+
+# mount/unmount enough times to trigger the /proc/self/mountinfo parsing rate limiting
+
+for ((i = 0; i < NUM_DIRS; i++)); do
+    mkdir "/tmp/meow${i}"
+done
+
+for ((i = 0; i < NUM_DIRS; i++)); do
+    mount -t tmpfs tmpfs "/tmp/meow${i}"
+done
+
+systemctl daemon-reload
+systemctl list-units -t mount tmp-meow* | grep -q tmp-meow
+
+for ((i = 0; i < NUM_DIRS; i++)); do
+    umount "/tmp/meow${i}"
+done
+
+# figure out if we have entered the rate limit state
+
+exited_rl=0
+timeout="$(date -ud "2 minutes" +%s)"
+while [[ $(date -u +%s) -le ${timeout} ]]; do
+    if journalctl -u init.scope | grep -q "(mount-monitor-dispatch) entered rate limit"; then
+        entered_rl=1
+        break
+    fi
+    sleep 5
+done
+
+# if the infra is slow we might not enter the rate limit state; in that case skip the exit check
+
+if [ "${entered_rl}" = "1" ]; then
+    exited_rl=0
+    timeout="$(date -ud "2 minutes" +%s)"
+    while [[ $(date -u +%s) -le ${timeout} ]]; do
+        if journalctl -u init.scope | grep -q "(mount-monitor-dispatch) left rate limit"; then
+            exited_rl=1
+            break
+        fi
+        sleep 5
+    done
+
+    if [ "${exited_rl}" = "0" ]; then
+        exit 24
+    fi
+fi
+
+# give some time for units to settle so we don't race between exiting the rate limit state and cleaning up the units
+
+sleep 60
+systemctl daemon-reload
+sleep 60
+
+# verify that the mount units are always cleaned up at the end
+
+if systemctl list-units -t mount tmp-meow* | grep -q tmp-meow; then
+    exit 42
+fi
+
+systemd-analyze log-level info
+
+echo OK >/testok
+
+exit 0