From c06b8d96f4f93ee72a0969129af631add3e76df7 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 6 Apr 2025 16:10:43 +0200 Subject: [PATCH] core: do not use pidref_hash_ops_free for Manager.watch_pids The PidRefs are in all cases owned by Unit.pids, and gets removed from Manager.watch_pids(_more) when the unit is destructed, via unit_unwatch_pidref(). This hasn't caused any issue because manager_clear_jobs_and_units() is called before destroying Manager.watch_pids(_more), but let's get this right. --- src/core/manager.h | 4 +++- src/core/unit.c | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/core/manager.h b/src/core/manager.h index aab8573c181..afc7d207744 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -253,7 +253,9 @@ struct Manager { * here: the first unit interested in a PID is stored in the hashmap 'watch_pids', keyed by the * PID. If there are other units interested too they'll be stored in a NULL-terminated array, stored * in the hashmap 'watch_pids_more', keyed by the PID. Thus to go through the full list of units - * interested in a PID we must look into both hashmaps. */ + * interested in a PID we must look into both hashmaps. + * + * NB: the ownership of PidRefs is held by Unit.pids! */ Hashmap *watch_pids; /* PidRef* → Unit* */ Hashmap *watch_pids_more; /* PidRef* → NUL terminated array of Unit* */ diff --git a/src/core/unit.c b/src/core/unit.c index ae977536b8b..a2eb1f70885 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2770,8 +2770,10 @@ int unit_watch_pidref(Unit *u, const PidRef *pid, bool exclusive) { if (exclusive) manager_unwatch_pidref(u->manager, pid); - if (set_contains(u->pids, pid)) /* early exit if already being watched */ + if (set_contains(u->pids, pid)) { /* early exit if already being watched */ + assert(!exclusive); return 0; + } r = pidref_dup(pid, &pid_dup); if (r < 0) @@ -2785,7 +2787,7 @@ int unit_watch_pidref(Unit *u, const PidRef *pid, bool exclusive) { pid = TAKE_PTR(pid_dup); /* continue with our copy now that we have installed it properly in our set */ /* Second, insert it into the simple global table, see if that works */ - r = hashmap_ensure_put(&u->manager->watch_pids, &pidref_hash_ops_free, pid, u); + r = hashmap_ensure_put(&u->manager->watch_pids, &pidref_hash_ops, pid, u); if (r != -EEXIST) return r; @@ -2811,7 +2813,7 @@ int unit_watch_pidref(Unit *u, const PidRef *pid, bool exclusive) { new_array[n+1] = NULL; /* Add or replace the old array */ - r = hashmap_ensure_replace(&u->manager->watch_pids_more, &pidref_hash_ops_free, old_pid ?: pid, new_array); + r = hashmap_ensure_replace(&u->manager->watch_pids_more, &pidref_hash_ops, old_pid ?: pid, new_array); if (r < 0) return r; -- 2.47.3