From: Mike Yuan <me@yhndnzj.com>
Date: Sun, 6 Apr 2025 14:10:43 +0000 (+0200)
Subject: core: do not use pidref_hash_ops_free for Manager.watch_pids
X-Git-Tag: v258-rc1~896
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c06b8d96f4f93ee72a0969129af631add3e76df7;p=thirdparty%2Fsystemd.git

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.
---

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;