From b1f6901d30eb8aecc551afc2fa8c782640dd7bd3 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Sun, 10 Sep 2023 14:49:16 +0200
Subject: [PATCH 1/5] core: port service_set_main_pid() to PidRef

---
 TODO               |  1 -
 src/core/service.c | 89 +++++++++++++++++++++++++++-------------------
 2 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/TODO b/TODO
index d54ff5cbb0..1d7da22058 100644
--- a/TODO
+++ b/TODO
@@ -176,7 +176,6 @@ Features:
   - unit_watch_pid() → unit_watch_pidref()
   - unit_kill_common()
   - unit_kill_context()
-  - service_set_main_pid()
   - actually wait for POLLIN on piref's pidfd in service logic
   - unit_main_pid() + unit_control_pid()
   - exec_spawn()
diff --git a/src/core/service.c b/src/core/service.c
index ecaeea63cf..a088af5196 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -179,38 +179,51 @@ static void service_unwatch_pid_file(Service *s) {
         s->pid_file_pathspec = mfree(s->pid_file_pathspec);
 }
 
+static int service_set_main_pidref(Service *s, PidRef *pidref) {
+        assert(s);
+
+        /* Takes ownership of the specified pidref on success, but not on failure. */
+
+        if (!pidref_is_set(pidref))
+                return -ESRCH;
+
+        if (pidref->pid <= 1)
+                return -EINVAL;
+
+        if (pidref->pid == getpid_cached())
+                return -EINVAL;
+
+        if (s->main_pid.pid == pidref->pid && s->main_pid_known) {
+                pidref_done(pidref);
+                return 0;
+        }
+
+        if (s->main_pid.pid != pidref->pid) {
+                service_unwatch_main_pid(s);
+                exec_status_start(&s->main_exec_status, pidref->pid);
+        }
+
+        s->main_pid = TAKE_PIDREF(*pidref);
+        s->main_pid_known = true;
+        s->main_pid_alien = pid_is_my_child(s->main_pid.pid) == 0;
+
+        if (s->main_pid_alien)
+                log_unit_warning(UNIT(s), "Supervising process "PID_FMT" which is not our child. We'll most likely not notice when it exits.", s->main_pid.pid);
+
+        return 0;
+}
+
 static int service_set_main_pid(Service *s, pid_t pid) {
         _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
         int r;
 
         assert(s);
 
-        if (pid <= 1)
-                return -EINVAL;
-
-        if (pid == getpid_cached())
-                return -EINVAL;
-
-        if (s->main_pid.pid == pid && s->main_pid_known)
-                return 0;
-
         r = pidref_set_pid(&pidref, pid);
         if (r < 0)
                 return r;
 
-        if (s->main_pid.pid != pid) {
-                service_unwatch_main_pid(s);
-                exec_status_start(&s->main_exec_status, pid);
-        }
-
-        s->main_pid = TAKE_PIDREF(pidref);
-        s->main_pid_known = true;
-        s->main_pid_alien = pid_is_my_child(pid) == 0;
-
-        if (s->main_pid_alien)
-                log_unit_warning(UNIT(s), "Supervising process "PID_FMT" which is not our child. We'll most likely not notice when it exits.", pid);
-
-        return 0;
+        return service_set_main_pidref(s, &pidref);
 }
 
 void service_release_socket_fd(Service *s) {
@@ -1108,6 +1121,7 @@ static int service_is_suitable_main_pid(Service *s, pid_t pid, int prio) {
 }
 
 static int service_load_pid_file(Service *s, bool may_warn) {
+        _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
         bool questionable_pid_file = false;
         _cleanup_free_ char *k = NULL;
         _cleanup_close_ int fd = -EBADF;
@@ -1149,7 +1163,11 @@ static int service_load_pid_file(Service *s, bool may_warn) {
         if (s->main_pid_known && pid == s->main_pid.pid)
                 return 0;
 
-        r = service_is_suitable_main_pid(s, pid, prio);
+        r = pidref_set_pid(&pidref, pid);
+        if (r < 0)
+                return log_unit_full_errno(UNIT(s), prio, r, "Failed to pin PID " PID_FMT ": %m", pid);
+
+        r = service_is_suitable_main_pid(s, pidref.pid, prio);
         if (r < 0)
                 return r;
         if (r == 0) {
@@ -1166,26 +1184,26 @@ static int service_load_pid_file(Service *s, bool may_warn) {
 
                 if (st.st_uid != 0)
                         return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(EPERM),
-                                                    "New main PID "PID_FMT" does not belong to service, and PID file is not owned by root. Refusing.", pid);
+                                                    "New main PID "PID_FMT" does not belong to service, and PID file is not owned by root. Refusing.", pidref.pid);
 
-                log_unit_debug(UNIT(s), "New main PID "PID_FMT" does not belong to service, but we'll accept it since PID file is owned by root.", pid);
+                log_unit_debug(UNIT(s), "New main PID "PID_FMT" does not belong to service, but we'll accept it since PID file is owned by root.", pidref.pid);
         }
 
         if (s->main_pid_known) {
-                log_unit_debug(UNIT(s), "Main PID changing: "PID_FMT" -> "PID_FMT, s->main_pid.pid, pid);
+                log_unit_debug(UNIT(s), "Main PID changing: "PID_FMT" -> "PID_FMT, s->main_pid.pid, pidref.pid);
 
                 service_unwatch_main_pid(s);
                 s->main_pid_known = false;
         } else
-                log_unit_debug(UNIT(s), "Main PID loaded: "PID_FMT, pid);
+                log_unit_debug(UNIT(s), "Main PID loaded: "PID_FMT, pidref.pid);
 
-        r = service_set_main_pid(s, pid);
+        r = service_set_main_pidref(s, &pidref);
         if (r < 0)
                 return r;
 
-        r = unit_watch_pid(UNIT(s), pid, /* exclusive= */ false);
+        r = unit_watch_pid(UNIT(s), s->main_pid.pid, /* exclusive= */ false);
         if (r < 0) /* FIXME: we need to do something here */
-                return log_unit_warning_errno(UNIT(s), r, "Failed to watch PID "PID_FMT" for service: %m", pid);
+                return log_unit_warning_errno(UNIT(s), r, "Failed to watch PID "PID_FMT" for service: %m", s->main_pid.pid);
 
         return 1;
 }
@@ -1212,10 +1230,10 @@ static void service_search_main_pid(Service *s) {
         if (service_set_main_pid(s, pid) < 0)
                 return;
 
-        r = unit_watch_pid(UNIT(s), pid, /* exclusive= */ false);
+        r = unit_watch_pid(UNIT(s), s->main_pid.pid, /* exclusive= */ false);
         if (r < 0)
                 /* FIXME: we need to do something here */
-                log_unit_warning_errno(UNIT(s), r, "Failed to watch PID "PID_FMT" from: %m", pid);
+                log_unit_warning_errno(UNIT(s), r, "Failed to watch PID "PID_FMT" from: %m", s->main_pid.pid);
 }
 
 static void service_set_state(Service *s, ServiceState state) {
@@ -2412,7 +2430,7 @@ static void service_enter_start(Service *s) {
                 /* For simple services we immediately start
                  * the START_POST binaries. */
 
-                (void) service_set_main_pid(s, pidref.pid);
+                (void) service_set_main_pidref(s, &pidref);
                 service_enter_start_post(s);
 
         } else  if (s->type == SERVICE_FORKING) {
@@ -2431,7 +2449,7 @@ static void service_enter_start(Service *s) {
                 /* For D-Bus services we know the main pid right away, but wait for the bus name to appear on the
                  * bus. 'notify' and 'exec' services are similar. */
 
-                (void) service_set_main_pid(s, pidref.pid);
+                (void) service_set_main_pidref(s, &pidref);
                 service_set_state(s, SERVICE_START);
         } else
                 assert_not_reached();
@@ -2700,8 +2718,7 @@ static void service_run_next_main(Service *s) {
         if (r < 0)
                 goto fail;
 
-        (void) service_set_main_pid(s, pidref.pid);
-
+        (void) service_set_main_pidref(s, &pidref);
         return;
 
 fail:

From 7901288ab18c9ffc296199064daf616d472e5796 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Sun, 10 Sep 2023 15:12:59 +0200
Subject: [PATCH 2/5] core: port over unit_kill_context() to PidRef

---
 TODO               |  1 -
 src/core/mount.c   |  6 +++---
 src/core/scope.c   |  4 +++-
 src/core/service.c |  4 ++--
 src/core/socket.c  |  6 +++---
 src/core/swap.c    | 13 +++++++------
 src/core/unit.c    | 32 ++++++++++++++++----------------
 src/core/unit.h    |  7 ++++---
 8 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/TODO b/TODO
index 1d7da22058..c75d65e0e4 100644
--- a/TODO
+++ b/TODO
@@ -175,7 +175,6 @@ Features:
   - pid_is_alive() → pidref_is_alive()
   - unit_watch_pid() → unit_watch_pidref()
   - unit_kill_common()
-  - unit_kill_context()
   - actually wait for POLLIN on piref's pidfd in service logic
   - unit_main_pid() + unit_control_pid()
   - exec_spawn()
diff --git a/src/core/mount.c b/src/core/mount.c
index 57045f3da4..c156a84c1d 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1036,9 +1036,9 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) {
                         UNIT(m),
                         &m->kill_context,
                         state_to_kill_operation(state),
-                        -1,
-                        m->control_pid.pid,
-                        false);
+                        /* main_pid= */ NULL,
+                        &m->control_pid,
+                        /* main_pid_alien= */ false);
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/scope.c b/src/core/scope.c
index a13c8ddf7a..31ea5c5483 100644
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -345,7 +345,9 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) {
                                 state != SCOPE_STOP_SIGTERM ? KILL_KILL :
                                 s->was_abandoned            ? KILL_TERMINATE_AND_LOG :
                                                               KILL_TERMINATE,
-                                -1, -1, false);
+                                /* main_pid= */ NULL,
+                                /* control_pid= */ NULL,
+                                /* main_pid_alien= */ false);
                 if (r < 0)
                         goto fail;
         }
diff --git a/src/core/service.c b/src/core/service.c
index a088af5196..34948fa923 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2166,8 +2166,8 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f
                         UNIT(s),
                         &s->kill_context,
                         kill_operation,
-                        s->main_pid.pid,
-                        s->control_pid.pid,
+                        &s->main_pid,
+                        &s->control_pid,
                         s->main_pid_alien);
         if (r < 0)
                 goto fail;
diff --git a/src/core/socket.c b/src/core/socket.c
index 9e7809721a..d885581247 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2117,9 +2117,9 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) {
                         UNIT(s),
                         &s->kill_context,
                         state_to_kill_operation(s, state),
-                        -1,
-                        s->control_pid.pid,
-                        false);
+                        /* main_pid= */ NULL,
+                        &s->control_pid,
+                        /* main_pid_alien= */ false);
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/swap.c b/src/core/swap.c
index c6b5f3471a..d8c0837576 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -763,12 +763,13 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) {
         if (s->result == SWAP_SUCCESS)
                 s->result = f;
 
-        r = unit_kill_context(UNIT(s),
-                              &s->kill_context,
-                              state_to_kill_operation(s, state),
-                              -1,
-                              s->control_pid.pid,
-                              false);
+        r = unit_kill_context(
+                        UNIT(s),
+                        &s->kill_context,
+                        state_to_kill_operation(s, state),
+                        /* main_pid= */ NULL,
+                        &s->control_pid,
+                        /* main_pid_alien= */ false);
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/unit.c b/src/core/unit.c
index 6f13348ebf..c542e4f504 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -4775,8 +4775,8 @@ int unit_kill_context(
                 Unit *u,
                 KillContext *c,
                 KillOperation k,
-                pid_t main_pid,
-                pid_t control_pid,
+                PidRef* main_pid,
+                PidRef* control_pid,
                 bool main_pid_alien) {
 
         bool wait_for_exit = false, send_sighup;
@@ -4803,40 +4803,40 @@ int unit_kill_context(
                 IN_SET(k, KILL_TERMINATE, KILL_TERMINATE_AND_LOG) &&
                 sig != SIGHUP;
 
-        if (main_pid > 0) {
+        if (pidref_is_set(main_pid)) {
                 if (log_func)
-                        log_func(main_pid, sig, u);
+                        log_func(main_pid->pid, sig, u);
 
-                r = kill_and_sigcont(main_pid, sig);
+                r = pidref_kill_and_sigcont(main_pid, sig);
                 if (r < 0 && r != -ESRCH) {
                         _cleanup_free_ char *comm = NULL;
-                        (void) get_process_comm(main_pid, &comm);
+                        (void) get_process_comm(main_pid->pid, &comm);
 
-                        log_unit_warning_errno(u, r, "Failed to kill main process " PID_FMT " (%s), ignoring: %m", main_pid, strna(comm));
+                        log_unit_warning_errno(u, r, "Failed to kill main process " PID_FMT " (%s), ignoring: %m", main_pid->pid, strna(comm));
                 } else {
                         if (!main_pid_alien)
                                 wait_for_exit = true;
 
                         if (r != -ESRCH && send_sighup)
-                                (void) kill(main_pid, SIGHUP);
+                                (void) pidref_kill(main_pid, SIGHUP);
                 }
         }
 
-        if (control_pid > 0) {
+        if (pidref_is_set(control_pid)) {
                 if (log_func)
-                        log_func(control_pid, sig, u);
+                        log_func(control_pid->pid, sig, u);
 
-                r = kill_and_sigcont(control_pid, sig);
+                r = pidref_kill_and_sigcont(control_pid, sig);
                 if (r < 0 && r != -ESRCH) {
                         _cleanup_free_ char *comm = NULL;
-                        (void) get_process_comm(control_pid, &comm);
+                        (void) get_process_comm(control_pid->pid, &comm);
 
-                        log_unit_warning_errno(u, r, "Failed to kill control process " PID_FMT " (%s), ignoring: %m", control_pid, strna(comm));
+                        log_unit_warning_errno(u, r, "Failed to kill control process " PID_FMT " (%s), ignoring: %m", control_pid->pid, strna(comm));
                 } else {
                         wait_for_exit = true;
 
                         if (r != -ESRCH && send_sighup)
-                                (void) kill(control_pid, SIGHUP);
+                                (void) pidref_kill(control_pid, SIGHUP);
                 }
         }
 
@@ -4845,7 +4845,7 @@ int unit_kill_context(
                 _cleanup_set_free_ Set *pid_set = NULL;
 
                 /* Exclude the main/control pids from being killed via the cgroup */
-                pid_set = unit_pid_set(main_pid, control_pid);
+                pid_set = unit_pid_set(main_pid ? main_pid->pid : 0, control_pid ? control_pid->pid : 0);
                 if (!pid_set)
                         return -ENOMEM;
 
@@ -4874,7 +4874,7 @@ int unit_kill_context(
                         if (send_sighup) {
                                 set_free(pid_set);
 
-                                pid_set = unit_pid_set(main_pid, control_pid);
+                                pid_set = unit_pid_set(main_pid ? main_pid->pid : 0, control_pid ? control_pid->pid : 0);
                                 if (!pid_set)
                                         return -ENOMEM;
 
diff --git a/src/core/unit.h b/src/core/unit.h
index c0710299a5..7ce61eb5fd 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -9,14 +9,15 @@
 #include "sd-id128.h"
 
 #include "bpf-program.h"
+#include "cgroup.h"
 #include "condition.h"
 #include "emergency-action.h"
 #include "install.h"
 #include "list.h"
-#include "show-status.h"
+#include "pidref.h"
 #include "set.h"
+#include "show-status.h"
 #include "unit-file.h"
-#include "cgroup.h"
 
 typedef struct UnitRef UnitRef;
 
@@ -992,7 +993,7 @@ char* unit_concat_strv(char **l, UnitWriteFlags flags);
 int unit_write_setting(Unit *u, UnitWriteFlags flags, const char *name, const char *data);
 int unit_write_settingf(Unit *u, UnitWriteFlags mode, const char *name, const char *format, ...) _printf_(4,5);
 
-int unit_kill_context(Unit *u, KillContext *c, KillOperation k, pid_t main_pid, pid_t control_pid, bool main_pid_alien);
+int unit_kill_context(Unit *u, KillContext *c, KillOperation k, PidRef *main_pid, PidRef *control_pid, bool main_pid_alien);
 
 int unit_make_transient(Unit *u);
 

From a0d1659c237c15c2858e00d0fe015c6ea8ab5ffe Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Sun, 10 Sep 2023 16:17:35 +0200
Subject: [PATCH 3/5] pidref: add sigqueue() helper

---
 src/basic/pidref.c | 28 ++++++++++++++++++++++++++++
 src/basic/pidref.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/src/basic/pidref.c b/src/basic/pidref.c
index f41460938c..e1ad697af0 100644
--- a/src/basic/pidref.c
+++ b/src/basic/pidref.c
@@ -6,6 +6,7 @@
 #include "parse-util.h"
 #include "pidref.h"
 #include "process-util.h"
+#include "signal-util.h"
 
 int pidref_set_pid(PidRef *pidref, pid_t pid) {
         int fd;
@@ -143,3 +144,30 @@ int pidref_kill_and_sigcont(PidRef *pidref, int sig) {
 
         return 0;
 }
+
+int pidref_sigqueue(PidRef *pidref, int sig, int value) {
+
+        if (!pidref)
+                return -ESRCH;
+
+        if (pidref->fd >= 0) {
+                siginfo_t si;
+
+                /* We can't use structured initialization here, since the structure contains various unions
+                 * and these fields lie in overlapping (carefully aligned) unions that LLVM is allergic to
+                 * allow assignments to */
+                zero(si);
+                si.si_signo = sig;
+                si.si_code = SI_QUEUE;
+                si.si_pid = getpid_cached();
+                si.si_uid = getuid();
+                si.si_value.sival_int = value;
+
+                return RET_NERRNO(pidfd_send_signal(pidref->fd, sig, &si, 0));
+        }
+
+        if (pidref->pid > 0)
+                return RET_NERRNO(sigqueue(pidref->pid, sig, (const union sigval) { .sival_int = value }));
+
+        return -ESRCH;
+}
diff --git a/src/basic/pidref.h b/src/basic/pidref.h
index 2411e510f1..6998a865e7 100644
--- a/src/basic/pidref.h
+++ b/src/basic/pidref.h
@@ -25,5 +25,6 @@ void pidref_done(PidRef *pidref);
 
 int pidref_kill(PidRef *pidref, int sig);
 int pidref_kill_and_sigcont(PidRef *pidref, int sig);
+int pidref_sigqueue(PidRef *pidfref, int sig, int value);
 
 #define TAKE_PIDREF(p) TAKE_GENERIC((p), PidRef, PIDREF_NULL)

From 37eb258e91c780fd6fe5e44110abd9da71dce6de Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Sun, 10 Sep 2023 16:25:02 +0200
Subject: [PATCH 4/5] core: port unit_main_pid() + unit_control_pid() to PidRef
 and drop unit_kill_common()

This ports over unit_main_pid() + unit_control_pid() to return PidRef*
pointers (which also means the underlying UnitVTable function pointers
are changed accordingly).

This then uses te functions to simplify the unit_kill() call, by
avoiding the kill() vtable indirection and instead just suing
unit_main_pid() and unit_control_pid() directly.
---
 TODO                    |  2 --
 src/core/dbus-service.c |  9 +++--
 src/core/dbus-unit.c    | 11 +++---
 src/core/mount.c        | 17 ++-------
 src/core/scope.c        |  6 ----
 src/core/service.c      | 25 +++----------
 src/core/slice.c        |  6 ----
 src/core/socket.c       | 13 ++-----
 src/core/swap.c         | 13 ++-----
 src/core/unit.c         | 78 +++++++++++++++++++----------------------
 src/core/unit.h         | 13 +++----
 11 files changed, 60 insertions(+), 133 deletions(-)

diff --git a/TODO b/TODO
index c75d65e0e4..bb12637798 100644
--- a/TODO
+++ b/TODO
@@ -174,9 +174,7 @@ Features:
   - pid_is_unwaited() → pidref_is_unwaited()
   - pid_is_alive() → pidref_is_alive()
   - unit_watch_pid() → unit_watch_pidref()
-  - unit_kill_common()
   - actually wait for POLLIN on piref's pidfd in service logic
-  - unit_main_pid() + unit_control_pid()
   - exec_spawn()
   - serialization of control/main pid in service, socket, mount, swap units
   - unit_fork_and_watch_rm_rf()
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index cc65a55990..5bc487bc39 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -134,7 +134,6 @@ static int bus_service_method_mount(sd_bus_message *message, void *userdata, sd_
         int read_only, make_file_or_directory;
         Unit *u = ASSERT_PTR(userdata);
         ExecContext *c;
-        pid_t unit_pid;
         int r;
 
         assert(message);
@@ -192,14 +191,14 @@ static int bus_service_method_mount(sd_bus_message *message, void *userdata, sd_
         if (!exec_needs_mount_namespace(c, NULL, unit_get_exec_runtime(u)))
                 return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Unit not running in private mount namespace, cannot activate bind mount");
 
-        unit_pid = unit_main_pid(u);
-        if (unit_pid == 0 || !UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u)))
+        PidRef* unit_pid = unit_main_pid(u);
+        if (!pidref_is_set(unit_pid) || !UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u)))
                 return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Unit is not running");
 
         propagate_directory = strjoina("/run/systemd/propagate/", u->id);
         if (is_image)
                 r = mount_image_in_namespace(
-                                unit_pid,
+                                unit_pid->pid,
                                 propagate_directory,
                                 "/run/systemd/incoming/",
                                 src, dest,
@@ -209,7 +208,7 @@ static int bus_service_method_mount(sd_bus_message *message, void *userdata, sd_
                                 c->mount_image_policy ?: &image_policy_service);
         else
                 r = bind_mount_in_namespace(
-                                unit_pid,
+                                unit_pid->pid,
                                 propagate_directory,
                                 "/run/systemd/incoming/",
                                 src, dest,
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 1f673fe26d..e9b446945a 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -1331,7 +1331,6 @@ int bus_unit_method_get_processes(sd_bus_message *message, void *userdata, sd_bu
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
         _cleanup_set_free_ Set *pids = NULL;
         Unit *u = userdata;
-        pid_t pid;
         int r;
 
         assert(message);
@@ -1359,16 +1358,16 @@ int bus_unit_method_get_processes(sd_bus_message *message, void *userdata, sd_bu
         }
 
         /* The main and control pids might live outside of the cgroup, hence fetch them separately */
-        pid = unit_main_pid(u);
-        if (pid > 0) {
-                r = append_process(reply, NULL, pid, pids);
+        PidRef *pid = unit_main_pid(u);
+        if (pidref_is_set(pid)) {
+                r = append_process(reply, NULL, pid->pid, pids);
                 if (r < 0)
                         return r;
         }
 
         pid = unit_control_pid(u);
-        if (pid > 0) {
-                r = append_process(reply, NULL, pid, pids);
+        if (pidref_is_set(pid)) {
+                r = append_process(reply, NULL, pid->pid, pids);
                 if (r < 0)
                         return r;
         }
diff --git a/src/core/mount.c b/src/core/mount.c
index c156a84c1d..4af2e6810e 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -2213,20 +2213,8 @@ static void mount_reset_failed(Unit *u) {
         m->clean_result = MOUNT_SUCCESS;
 }
 
-static int mount_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) {
-        Mount *m = MOUNT(u);
-
-        assert(m);
-
-        return unit_kill_common(u, who, signo, code, value, -1, m->control_pid.pid, error);
-}
-
-static int mount_control_pid(Unit *u) {
-        Mount *m = MOUNT(u);
-
-        assert(m);
-
-        return m->control_pid.pid;
+static PidRef* mount_control_pid(Unit *u) {
+        return &ASSERT_PTR(MOUNT(u))->control_pid;
 }
 
 static int mount_clean(Unit *u, ExecCleanMask mask) {
@@ -2358,7 +2346,6 @@ const UnitVTable mount_vtable = {
         .stop = mount_stop,
         .reload = mount_reload,
 
-        .kill = mount_kill,
         .clean = mount_clean,
         .can_clean = mount_can_clean,
 
diff --git a/src/core/scope.c b/src/core/scope.c
index 31ea5c5483..13907855c5 100644
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -533,10 +533,6 @@ static void scope_reset_failed(Unit *u) {
         s->result = SCOPE_SUCCESS;
 }
 
-static int scope_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) {
-        return unit_kill_common(u, who, signo, code, value, -1, -1, error);
-}
-
 static int scope_get_timeout(Unit *u, usec_t *timeout) {
         Scope *s = SCOPE(u);
         usec_t t;
@@ -830,8 +826,6 @@ const UnitVTable scope_vtable = {
         .start = scope_start,
         .stop = scope_stop,
 
-        .kill = scope_kill,
-
         .freeze = unit_freeze_vtable_common,
         .thaw = unit_thaw_vtable_common,
 
diff --git a/src/core/service.c b/src/core/service.c
index 34948fa923..0ecfe46cfd 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -4825,28 +4825,12 @@ static void service_reset_failed(Unit *u) {
         s->flush_n_restarts = false;
 }
 
-static int service_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) {
-        Service *s = SERVICE(u);
-
-        assert(s);
-
-        return unit_kill_common(u, who, signo, code, value, s->main_pid.pid, s->control_pid.pid, error);
+static PidRef* service_main_pid(Unit *u) {
+        return &ASSERT_PTR(SERVICE(u))->main_pid;
 }
 
-static int service_main_pid(Unit *u) {
-        Service *s = SERVICE(u);
-
-        assert(s);
-
-        return s->main_pid.pid;
-}
-
-static int service_control_pid(Unit *u) {
-        Service *s = SERVICE(u);
-
-        assert(s);
-
-        return s->control_pid.pid;
+static PidRef* service_control_pid(Unit *u) {
+        return &ASSERT_PTR(SERVICE(u))->control_pid;
 }
 
 static bool service_needs_console(Unit *u) {
@@ -5163,7 +5147,6 @@ const UnitVTable service_vtable = {
 
         .can_reload = service_can_reload,
 
-        .kill = service_kill,
         .clean = service_clean,
         .can_clean = service_can_clean,
 
diff --git a/src/core/slice.c b/src/core/slice.c
index c6d142cebb..c7701b4017 100644
--- a/src/core/slice.c
+++ b/src/core/slice.c
@@ -247,10 +247,6 @@ static int slice_stop(Unit *u) {
         return 1;
 }
 
-static int slice_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) {
-        return unit_kill_common(u, who, signo, code, value, -1, -1, error);
-}
-
 static int slice_serialize(Unit *u, FILE *f, FDSet *fds) {
         Slice *s = SLICE(u);
 
@@ -436,8 +432,6 @@ const UnitVTable slice_vtable = {
         .start = slice_start,
         .stop = slice_stop,
 
-        .kill = slice_kill,
-
         .freeze = slice_freeze,
         .thaw = slice_thaw,
         .can_freeze = slice_can_freeze,
diff --git a/src/core/socket.c b/src/core/socket.c
index d885581247..861e580b3f 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -3382,10 +3382,6 @@ static void socket_trigger_notify(Unit *u, Unit *other) {
                 socket_set_state(s, SOCKET_RUNNING);
 }
 
-static int socket_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) {
-        return unit_kill_common(u, who, signo, code, value, -1, SOCKET(u)->control_pid.pid, error);
-}
-
 static int socket_get_timeout(Unit *u, usec_t *timeout) {
         Socket *s = SOCKET(u);
         usec_t t;
@@ -3414,12 +3410,8 @@ char *socket_fdname(Socket *s) {
         return s->fdname ?: UNIT(s)->id;
 }
 
-static int socket_control_pid(Unit *u) {
-        Socket *s = SOCKET(u);
-
-        assert(s);
-
-        return s->control_pid.pid;
+static PidRef *socket_control_pid(Unit *u) {
+        return &ASSERT_PTR(SOCKET(u))->control_pid;
 }
 
 static int socket_clean(Unit *u, ExecCleanMask mask) {
@@ -3576,7 +3568,6 @@ const UnitVTable socket_vtable = {
         .start = socket_start,
         .stop = socket_stop,
 
-        .kill = socket_kill,
         .clean = socket_clean,
         .can_clean = socket_can_clean,
 
diff --git a/src/core/swap.c b/src/core/swap.c
index d8c0837576..e06de629da 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -1476,10 +1476,6 @@ static void swap_reset_failed(Unit *u) {
         s->clean_result = SWAP_SUCCESS;
 }
 
-static int swap_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) {
-        return unit_kill_common(u, who, signo, code, value, -1, SWAP(u)->control_pid.pid, error);
-}
-
 static int swap_get_timeout(Unit *u, usec_t *timeout) {
         Swap *s = SWAP(u);
         usec_t t;
@@ -1516,12 +1512,8 @@ static bool swap_supported(void) {
         return supported;
 }
 
-static int swap_control_pid(Unit *u) {
-        Swap *s = SWAP(u);
-
-        assert(s);
-
-        return s->control_pid.pid;
+static PidRef* swap_control_pid(Unit *u) {
+        return &ASSERT_PTR(SWAP(u))->control_pid;
 }
 
 static int swap_clean(Unit *u, ExecCleanMask mask) {
@@ -1638,7 +1630,6 @@ const UnitVTable swap_vtable = {
         .start = swap_start,
         .stop = swap_stop,
 
-        .kill = swap_kill,
         .clean = swap_clean,
         .can_clean = swap_can_clean,
 
diff --git a/src/core/unit.c b/src/core/unit.c
index c542e4f504..c17e099018 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -2943,7 +2943,7 @@ void unit_unwatch_all_pids(Unit *u) {
 }
 
 static void unit_tidy_watch_pids(Unit *u) {
-        pid_t except1, except2;
+        PidRef *except1, *except2;
         void *e;
 
         assert(u);
@@ -2956,7 +2956,8 @@ static void unit_tidy_watch_pids(Unit *u) {
         SET_FOREACH(e, u->pids) {
                 pid_t pid = PTR_TO_PID(e);
 
-                if (pid == except1 || pid == except2)
+                if ((pidref_is_set(except1) && pid == except1->pid) ||
+                    (pidref_is_set(except2) && pid == except2->pid))
                         continue;
 
                 if (!pid_is_unwaited(pid))
@@ -3958,18 +3959,6 @@ bool unit_will_restart(Unit *u) {
         return UNIT_VTABLE(u)->will_restart(u);
 }
 
-int unit_kill(Unit *u, KillWho w, int signo, int code, int value, sd_bus_error *error) {
-        assert(u);
-        assert(w >= 0 && w < _KILL_WHO_MAX);
-        assert(SIGNAL_VALID(signo));
-        assert(IN_SET(code, SI_USER, SI_QUEUE));
-
-        if (!UNIT_VTABLE(u)->kill)
-                return -EOPNOTSUPP;
-
-        return UNIT_VTABLE(u)->kill(u, w, signo, code, value, error);
-}
-
 void unit_notify_cgroup_oom(Unit *u, bool managed_oom) {
         assert(u);
 
@@ -4012,35 +4001,34 @@ static int kill_common_log(pid_t pid, int signo, void *userdata) {
         return 1;
 }
 
-static int kill_or_sigqueue(pid_t pid, int signo, int code, int value) {
-        assert(pid > 0);
+static int kill_or_sigqueue(PidRef* pidref, int signo, int code, int value) {
+        assert(pidref_is_set(pidref));
         assert(SIGNAL_VALID(signo));
 
         switch (code) {
 
         case SI_USER:
-                log_debug("Killing " PID_FMT " with signal SIG%s.", pid, signal_to_string(signo));
-                return RET_NERRNO(kill(pid, signo));
+                log_debug("Killing " PID_FMT " with signal SIG%s.", pidref->pid, signal_to_string(signo));
+                return pidref_kill(pidref, signo);
 
         case SI_QUEUE:
-                log_debug("Enqueuing value %i to " PID_FMT " on signal SIG%s.", value, pid, signal_to_string(signo));
-                return RET_NERRNO(sigqueue(pid, signo, (const union sigval) { .sival_int = value }));
+                log_debug("Enqueuing value %i to " PID_FMT " on signal SIG%s.", value, pidref->pid, signal_to_string(signo));
+                return pidref_sigqueue(pidref, signo, value);
 
         default:
                 assert_not_reached();
         }
 }
 
-int unit_kill_common(
+int unit_kill(
                 Unit *u,
                 KillWho who,
                 int signo,
                 int code,
                 int value,
-                pid_t main_pid,
-                pid_t control_pid,
                 sd_bus_error *error) {
 
+        PidRef *main_pid, *control_pid;
         bool killed = false;
         int ret = 0, r;
 
@@ -4054,24 +4042,30 @@ int unit_kill_common(
         assert(SIGNAL_VALID(signo));
         assert(IN_SET(code, SI_USER, SI_QUEUE));
 
+        main_pid = unit_main_pid(u);
+        control_pid = unit_control_pid(u);
+
+        if (!UNIT_HAS_CGROUP_CONTEXT(u) && !main_pid && !control_pid)
+                return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Unit type does not support process killing.");
+
         if (IN_SET(who, KILL_MAIN, KILL_MAIN_FAIL)) {
-                if (main_pid < 0)
+                if (!main_pid)
                         return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_PROCESS, "%s units have no main processes", unit_type_to_string(u->type));
-                if (main_pid == 0)
+                if (!pidref_is_set(main_pid))
                         return sd_bus_error_set_const(error, BUS_ERROR_NO_SUCH_PROCESS, "No main process to kill");
         }
 
         if (IN_SET(who, KILL_CONTROL, KILL_CONTROL_FAIL)) {
-                if (control_pid < 0)
+                if (!control_pid)
                         return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_PROCESS, "%s units have no control processes", unit_type_to_string(u->type));
-                if (control_pid == 0)
+                if (!pidref_is_set(control_pid))
                         return sd_bus_error_set_const(error, BUS_ERROR_NO_SUCH_PROCESS, "No control process to kill");
         }
 
-        if (control_pid > 0 &&
+        if (pidref_is_set(control_pid) &&
             IN_SET(who, KILL_CONTROL, KILL_CONTROL_FAIL, KILL_ALL, KILL_ALL_FAIL)) {
                 _cleanup_free_ char *comm = NULL;
-                (void) get_process_comm(control_pid, &comm);
+                (void) get_process_comm(control_pid->pid, &comm);
 
                 r = kill_or_sigqueue(control_pid, signo, code, value);
                 if (r < 0) {
@@ -4081,23 +4075,23 @@ int unit_kill_common(
                         sd_bus_error_set_errnof(
                                         error, r,
                                         "Failed to send signal SIG%s to control process " PID_FMT " (%s): %m",
-                                        signal_to_string(signo), control_pid, strna(comm));
+                                        signal_to_string(signo), control_pid->pid, strna(comm));
                         log_unit_warning_errno(
                                         u, r,
                                         "Failed to send signal SIG%s to control process " PID_FMT " (%s) on client request: %m",
-                                        signal_to_string(signo), control_pid, strna(comm));
+                                        signal_to_string(signo), control_pid->pid, strna(comm));
                 } else {
                         log_unit_info(u, "Sent signal SIG%s to control process " PID_FMT " (%s) on client request.",
-                                      signal_to_string(signo), control_pid, strna(comm));
+                                      signal_to_string(signo), control_pid->pid, strna(comm));
                         killed = true;
                 }
         }
 
-        if (main_pid > 0 &&
+        if (pidref_is_set(main_pid) &&
             IN_SET(who, KILL_MAIN, KILL_MAIN_FAIL, KILL_ALL, KILL_ALL_FAIL)) {
 
                 _cleanup_free_ char *comm = NULL;
-                (void) get_process_comm(main_pid, &comm);
+                (void) get_process_comm(main_pid->pid, &comm);
 
                 r = kill_or_sigqueue(main_pid, signo, code, value);
                 if (r < 0) {
@@ -4107,17 +4101,17 @@ int unit_kill_common(
                                 sd_bus_error_set_errnof(
                                                 error, r,
                                                 "Failed to send signal SIG%s to main process " PID_FMT " (%s): %m",
-                                                signal_to_string(signo), main_pid, strna(comm));
+                                                signal_to_string(signo), main_pid->pid, strna(comm));
                         }
 
                         log_unit_warning_errno(
                                         u, r,
                                         "Failed to send signal SIG%s to main process " PID_FMT " (%s) on client request: %m",
-                                        signal_to_string(signo), main_pid, strna(comm));
+                                        signal_to_string(signo), main_pid->pid, strna(comm));
 
                 } else {
                         log_unit_info(u, "Sent signal SIG%s to main process " PID_FMT " (%s) on client request.",
-                                      signal_to_string(signo), main_pid, strna(comm));
+                                      signal_to_string(signo), main_pid->pid, strna(comm));
                         killed = true;
                 }
         }
@@ -4129,7 +4123,7 @@ int unit_kill_common(
                 _cleanup_set_free_ Set *pid_set = NULL;
 
                 /* Exclude the main/control pids from being killed via the cgroup */
-                pid_set = unit_pid_set(main_pid, control_pid);
+                pid_set = unit_pid_set(main_pid ? main_pid->pid : 0, control_pid ? control_pid->pid : 0);
                 if (!pid_set)
                         return log_oom();
 
@@ -5120,22 +5114,22 @@ bool unit_is_pristine(Unit *u) {
                !u->merged_into;
 }
 
-pid_t unit_control_pid(Unit *u) {
+PidRef* unit_control_pid(Unit *u) {
         assert(u);
 
         if (UNIT_VTABLE(u)->control_pid)
                 return UNIT_VTABLE(u)->control_pid(u);
 
-        return 0;
+        return NULL;
 }
 
-pid_t unit_main_pid(Unit *u) {
+PidRef* unit_main_pid(Unit *u) {
         assert(u);
 
         if (UNIT_VTABLE(u)->main_pid)
                 return UNIT_VTABLE(u)->main_pid(u);
 
-        return 0;
+        return NULL;
 }
 
 static void unit_unref_uid_internal(
diff --git a/src/core/unit.h b/src/core/unit.h
index 7ce61eb5fd..d0d713f612 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -625,8 +625,6 @@ typedef struct UnitVTable {
         int (*stop)(Unit *u);
         int (*reload)(Unit *u);
 
-        int (*kill)(Unit *u, KillWho w, int signo, int code, int value, sd_bus_error *error);
-
         /* Clear out the various runtime/state/cache/logs/configuration data */
         int (*clean)(Unit *u, ExecCleanMask m);
 
@@ -720,10 +718,10 @@ typedef struct UnitVTable {
         usec_t (*get_timeout_start_usec)(Unit *u);
 
         /* Returns the main PID if there is any defined, or 0. */
-        pid_t (*main_pid)(Unit *u);
+        PidRef* (*main_pid)(Unit *u);
 
-        /* Returns the main PID if there is any defined, or 0. */
-        pid_t (*control_pid)(Unit *u);
+        /* Returns the control PID if there is any defined, or 0. */
+        PidRef* (*control_pid)(Unit *u);
 
         /* Returns true if the unit currently needs access to the console */
         bool (*needs_console)(Unit *u);
@@ -914,7 +912,6 @@ int unit_stop(Unit *u);
 int unit_reload(Unit *u);
 
 int unit_kill(Unit *u, KillWho w, int signo, int code, int value, sd_bus_error *error);
-int unit_kill_common(Unit *u, KillWho who, int signo, int code, int value, pid_t main_pid, pid_t control_pid, sd_bus_error *error);
 
 void unit_notify_cgroup_oom(Unit *u, bool managed_oom);
 
@@ -1007,8 +1004,8 @@ bool unit_is_unneeded(Unit *u);
 bool unit_is_upheld_by_active(Unit *u, Unit **ret_culprit);
 bool unit_is_bound_by_inactive(Unit *u, Unit **ret_culprit);
 
-pid_t unit_control_pid(Unit *u);
-pid_t unit_main_pid(Unit *u);
+PidRef* unit_control_pid(Unit *u);
+PidRef* unit_main_pid(Unit *u);
 
 void unit_warn_if_dir_nonempty(Unit *u, const char* where);
 int unit_fail_if_noncanonical(Unit *u, const char* where);

From 4775b55db0065463282922c3560e6e391de93501 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Sun, 10 Sep 2023 16:41:43 +0200
Subject: [PATCH 5/5] core: port unit_fork_helper_process() and
 unit_fork_and_watch_rm_rf() to PidRef

---
 TODO               |  3 +--
 src/core/mount.c   |  7 +------
 src/core/scope.c   |  6 +++---
 src/core/service.c |  7 +------
 src/core/socket.c  | 26 ++++++++------------------
 src/core/swap.c    |  7 +------
 src/core/unit.c    | 30 +++++++++++++++++++++++-------
 src/core/unit.h    |  4 ++--
 8 files changed, 40 insertions(+), 50 deletions(-)

diff --git a/TODO b/TODO
index bb12637798..349e31f1f7 100644
--- a/TODO
+++ b/TODO
@@ -174,10 +174,9 @@ Features:
   - pid_is_unwaited() → pidref_is_unwaited()
   - pid_is_alive() → pidref_is_alive()
   - unit_watch_pid() → unit_watch_pidref()
-  - actually wait for POLLIN on piref's pidfd in service logic
+  - actually wait for POLLIN on pidref's pidfd in service logic
   - exec_spawn()
   - serialization of control/main pid in service, socket, mount, swap units
-  - unit_fork_and_watch_rm_rf()
   - cg_pid_get_unit()
   - openpt_allocate_in_namespace()
   - scope dbus PIDs property needs to gain PIDFDs companion
diff --git a/src/core/mount.c b/src/core/mount.c
index 4af2e6810e..08c2ddf915 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -2220,7 +2220,6 @@ static PidRef* mount_control_pid(Unit *u) {
 static int mount_clean(Unit *u, ExecCleanMask mask) {
         _cleanup_strv_free_ char **l = NULL;
         Mount *m = MOUNT(u);
-        pid_t pid;
         int r;
 
         assert(m);
@@ -2245,11 +2244,7 @@ static int mount_clean(Unit *u, ExecCleanMask mask) {
         if (r < 0)
                 goto fail;
 
-        r = unit_fork_and_watch_rm_rf(u, l, &pid);
-        if (r < 0)
-                goto fail;
-
-        r = pidref_set_pid(&m->control_pid, pid);
+        r = unit_fork_and_watch_rm_rf(u, l, &m->control_pid);
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/scope.c b/src/core/scope.c
index 13907855c5..eb3ffba084 100644
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -372,8 +372,8 @@ fail:
 }
 
 static int scope_enter_start_chown(Scope *s) {
+        _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
         Unit *u = UNIT(s);
-        pid_t pid;
         int r;
 
         assert(s);
@@ -383,7 +383,7 @@ static int scope_enter_start_chown(Scope *s) {
         if (r < 0)
                 return r;
 
-        r = unit_fork_helper_process(u, "(sd-chown-cgroup)", &pid);
+        r = unit_fork_helper_process(u, "(sd-chown-cgroup)", &pidref);
         if (r < 0)
                 goto fail;
 
@@ -420,7 +420,7 @@ static int scope_enter_start_chown(Scope *s) {
                 _exit(EXIT_SUCCESS);
         }
 
-        r = unit_watch_pid(UNIT(s), pid, true);
+        r = unit_watch_pid(UNIT(s), pidref.pid, /* exclusive= */ true);
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/service.c b/src/core/service.c
index 0ecfe46cfd..05a2bb6ddf 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -4890,7 +4890,6 @@ static int service_clean(Unit *u, ExecCleanMask mask) {
         _cleanup_strv_free_ char **l = NULL;
         bool may_clean_fdstore = false;
         Service *s = SERVICE(u);
-        pid_t pid;
         int r;
 
         assert(s);
@@ -4931,11 +4930,7 @@ static int service_clean(Unit *u, ExecCleanMask mask) {
         if (r < 0)
                 goto fail;
 
-        r = unit_fork_and_watch_rm_rf(u, l, &pid);
-        if (r < 0)
-                goto fail;
-
-        r = pidref_set_pid(&s->control_pid, pid);
+        r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid);
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/socket.c b/src/core/socket.c
index 861e580b3f..3304af831d 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1503,9 +1503,9 @@ static int socket_address_listen_in_cgroup(
                 const SocketAddress *address,
                 const char *label) {
 
+        _cleanup_(pidref_done) PidRef pid = PIDREF_NULL;
         _cleanup_close_pair_ int pair[2] = PIPE_EBADF;
         int fd, r;
-        pid_t pid;
 
         assert(s);
         assert(address);
@@ -1597,7 +1597,7 @@ static int socket_address_listen_in_cgroup(
         fd = receive_one_fd(pair[0], 0);
 
         /* We synchronously wait for the helper, as it shouldn't be slow */
-        r = wait_for_terminate_and_check("(sd-listen)", pid, WAIT_LOG_ABNORMAL);
+        r = wait_for_terminate_and_check("(sd-listen)", pid.pid, WAIT_LOG_ABNORMAL);
         if (r < 0) {
                 safe_close(fd);
                 return r;
@@ -1968,8 +1968,7 @@ static int socket_spawn(Socket *s, ExecCommand *c, PidRef *ret_pid) {
 }
 
 static int socket_chown(Socket *s, PidRef *ret_pid) {
-        _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
-        pid_t pid;
+        _cleanup_(pidref_done) PidRef pid = PIDREF_NULL;
         int r;
 
         assert(s);
@@ -2030,15 +2029,11 @@ static int socket_chown(Socket *s, PidRef *ret_pid) {
                 _exit(EXIT_SUCCESS);
         }
 
-        r = pidref_set_pid(&pidref, pid);
+        r = unit_watch_pid(UNIT(s), pid.pid, /* exclusive= */ true);
         if (r < 0)
                 return r;
 
-        r = unit_watch_pid(UNIT(s), pidref.pid, /* exclusive= */ true);
-        if (r < 0)
-                return r;
-
-        *ret_pid = TAKE_PIDREF(pidref);
+        *ret_pid = TAKE_PIDREF(pid);
         return 0;
 }
 
@@ -2982,9 +2977,9 @@ static int socket_accept_do(Socket *s, int fd) {
 }
 
 static int socket_accept_in_cgroup(Socket *s, SocketPort *p, int fd) {
+        _cleanup_(pidref_done) PidRef pid = PIDREF_NULL;
         _cleanup_close_pair_ int pair[2] = PIPE_EBADF;
         int cfd, r;
-        pid_t pid;
 
         assert(s);
         assert(p);
@@ -3034,7 +3029,7 @@ static int socket_accept_in_cgroup(Socket *s, SocketPort *p, int fd) {
         cfd = receive_one_fd(pair[0], 0);
 
         /* We synchronously wait for the helper, as it shouldn't be slow */
-        r = wait_for_terminate_and_check("(sd-accept)", pid, WAIT_LOG_ABNORMAL);
+        r = wait_for_terminate_and_check("(sd-accept)", pid.pid, WAIT_LOG_ABNORMAL);
         if (r < 0) {
                 safe_close(cfd);
                 return r;
@@ -3417,7 +3412,6 @@ static PidRef *socket_control_pid(Unit *u) {
 static int socket_clean(Unit *u, ExecCleanMask mask) {
         _cleanup_strv_free_ char **l = NULL;
         Socket *s = SOCKET(u);
-        pid_t pid;
         int r;
 
         assert(s);
@@ -3442,11 +3436,7 @@ static int socket_clean(Unit *u, ExecCleanMask mask) {
         if (r < 0)
                 goto fail;
 
-        r = unit_fork_and_watch_rm_rf(u, l, &pid);
-        if (r < 0)
-                goto fail;
-
-        r = pidref_set_pid(&s->control_pid, pid);
+        r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid);
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/swap.c b/src/core/swap.c
index e06de629da..aa04b99462 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -1519,7 +1519,6 @@ static PidRef* swap_control_pid(Unit *u) {
 static int swap_clean(Unit *u, ExecCleanMask mask) {
         _cleanup_strv_free_ char **l = NULL;
         Swap *s = SWAP(u);
-        pid_t pid;
         int r;
 
         assert(s);
@@ -1544,11 +1543,7 @@ static int swap_clean(Unit *u, ExecCleanMask mask) {
         if (r < 0)
                 goto fail;
 
-        r = unit_fork_and_watch_rm_rf(u, l, &pid);
-        if (r < 0)
-                goto fail;
-
-        r = pidref_set_pid(&s->control_pid, pid);
+        r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid);
         if (r < 0)
                 goto fail;
 
diff --git a/src/core/unit.c b/src/core/unit.c
index c17e099018..5fd66e7b6e 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -5319,7 +5319,8 @@ int unit_set_exec_params(Unit *u, ExecParameters *p) {
         return 0;
 }
 
-int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret) {
+int unit_fork_helper_process(Unit *u, const char *name, PidRef *ret) {
+        pid_t pid;
         int r;
 
         assert(u);
@@ -5330,9 +5331,24 @@ int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret) {
 
         (void) unit_realize_cgroup(u);
 
-        r = safe_fork(name, FORK_REOPEN_LOG|FORK_DEATHSIG, ret);
-        if (r != 0)
+        r = safe_fork(name, FORK_REOPEN_LOG|FORK_DEATHSIG, &pid);
+        if (r < 0)
                 return r;
+        if (r > 0) {
+                _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
+                int q;
+
+                /* Parent */
+
+                q = pidref_set_pid(&pidref, pid);
+                if (q < 0)
+                        return q;
+
+                *ret = TAKE_PIDREF(pidref);
+                return r;
+        }
+
+        /* Child */
 
         (void) default_signals(SIGNALS_CRASH_HANDLER, SIGNALS_IGNORE);
         (void) ignore_signals(SIGPIPE);
@@ -5348,8 +5364,8 @@ int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret) {
         return 0;
 }
 
-int unit_fork_and_watch_rm_rf(Unit *u, char **paths, pid_t *ret_pid) {
-        pid_t pid;
+int unit_fork_and_watch_rm_rf(Unit *u, char **paths, PidRef *ret_pid) {
+        _cleanup_(pidref_done) PidRef pid = PIDREF_NULL;
         int r;
 
         assert(u);
@@ -5372,11 +5388,11 @@ int unit_fork_and_watch_rm_rf(Unit *u, char **paths, pid_t *ret_pid) {
                 _exit(ret);
         }
 
-        r = unit_watch_pid(u, pid, true);
+        r = unit_watch_pid(u, pid.pid, /* exclusive= */ true);
         if (r < 0)
                 return r;
 
-        *ret_pid = pid;
+        *ret_pid = TAKE_PIDREF(pid);
         return 0;
 }
 
diff --git a/src/core/unit.h b/src/core/unit.h
index d0d713f612..f1a80cc891 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -1024,8 +1024,8 @@ bool unit_shall_confirm_spawn(Unit *u);
 
 int unit_set_exec_params(Unit *s, ExecParameters *p);
 
-int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret);
-int unit_fork_and_watch_rm_rf(Unit *u, char **paths, pid_t *ret_pid);
+int unit_fork_helper_process(Unit *u, const char *name, PidRef *ret);
+int unit_fork_and_watch_rm_rf(Unit *u, char **paths, PidRef *ret);
 
 void unit_remove_dependencies(Unit *u, UnitDependencyMask mask);