SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

pidref: Add pidref_wait_for_terminate_full()

Browse Source 
We can use this to replace wait_for_terminate_with_timeout(). Because
we poll a pidfd, we don't need to block SIGCHLD anymore in umount.c.
This commit is contained in:
Daan De Meyer
2025-12-20 15:49:26 +01:00
parent 9bd833ee6e
commit e74b571004
9 changed files with 122 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/pidref.c
+36 -18
View File
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */
#include <poll.h>
#include <sys/wait.h>
#include <unistd.h>
@@ -8,12 +9,14 @@
#include "fd-util.h"
#include "format-util.h"
#include "hash-funcs.h"
#include "io-util.h"
#include "log.h"
#include "parse-util.h"
#include "pidfd-util.h"
#include "pidref.h"
#include "process-util.h"
#include "siphash24.h"
#include "time-util.h"
int pidref_acquire_pidfd_id(PidRef *pidref) {
int r;
@@ -449,7 +452,8 @@ bool pidref_is_self(PidRef *pidref) {
return pidref->fd_id == self_id;
}
int pidref_wait(PidRef *pidref, siginfo_t *ret, int options) {
int pidref_wait_for_terminate_full(PidRef *pidref, usec_t timeout, siginfo_t *ret) {
siginfo_t si = {};
int r;
if (!pidref_is_set(pidref))
@@ -461,13 +465,37 @@ int pidref_wait(PidRef *pidref, siginfo_t *ret, int options) {
if (pidref->pid == 1 || pidref_is_self(pidref))
return -ECHILD;
siginfo_t si = {};
if (pidref->fd >= 0)
r = RET_NERRNO(waitid(P_PIDFD, pidref->fd, &si, options));
else
r = RET_NERRNO(waitid(P_PID, pidref->pid, &si, options));
if (r < 0)
return r;
if (timeout != USEC_INFINITY && pidref->fd < 0)
return -ENOMEDIUM;
usec_t ts = timeout == USEC_INFINITY ? USEC_INFINITY : now(CLOCK_MONOTONIC) + timeout;
for (;;) {
if (timeout != USEC_INFINITY) {
usec_t left = usec_sub_unsigned(ts, now(CLOCK_MONOTONIC));
if (left == 0)
return -ETIMEDOUT;
r = fd_wait_for_event(pidref->fd, POLLIN, left);
if (r == 0)
return -ETIMEDOUT;
if (r == -EINTR)
continue;
if (r < 0)
return r;
}
if (pidref->fd >= 0)
r = RET_NERRNO(waitid(P_PIDFD, pidref->fd, &si, WEXITED));
else
r = RET_NERRNO(waitid(P_PID, pidref->pid, &si, WEXITED));
if (r == -EINTR)
continue;
if (r < 0)
return r;
break;
}
if (ret)
*ret = si;
@@ -475,16 +503,6 @@ int pidref_wait(PidRef *pidref, siginfo_t *ret, int options) {
return 0;
}
int pidref_wait_for_terminate(PidRef *pidref, siginfo_t *ret) {
int r;
for (;;) {
r = pidref_wait(pidref, ret, WEXITED);
if (r != -EINTR)
return r;
}
}
bool pidref_is_automatic(const PidRef *pidref) {
return pidref && pid_is_automatic(pidref->pid);
}
src/basic/pidref.h
+12 -2
View File
@@ -96,8 +96,10 @@ int pidref_kill(const PidRef *pidref, int sig);
int pidref_kill_and_sigcont(const PidRef *pidref, int sig);
int pidref_sigqueue(const PidRef *pidref, int sig, int value);
int pidref_wait(PidRef *pidref, siginfo_t *ret, int options);
int pidref_wait_for_terminate(PidRef *pidref, siginfo_t *ret);
int pidref_wait_for_terminate_full(PidRef *pidref, usec_t timeout, siginfo_t *ret);
static inline int pidref_wait_for_terminate(PidRef *pidref, siginfo_t *ret) {
return pidref_wait_for_terminate_full(pidref, USEC_INFINITY, ret);
}
static inline void pidref_done_sigkill_wait(PidRef *pidref) {
if (!pidref_is_set(pidref))
@@ -108,6 +110,14 @@ static inline void pidref_done_sigkill_wait(PidRef *pidref) {
pidref_done(pidref);
}
static inline void pidref_done_sigkill_nowait(PidRef *pidref) {
if (!pidref_is_set(pidref))
return;
(void) pidref_kill(pidref, SIGKILL);
pidref_done(pidref);
}
int pidref_verify(const PidRef *pidref);
#define TAKE_PIDREF(p) TAKE_GENERIC((p), PidRef, PIDREF_NULL)
src/basic/process-util.c
-63
View File
@@ -912,69 +912,6 @@ int wait_for_terminate_and_check(const char *name, pid_t pid, WaitFlags flags) {
return pidref_wait_for_terminate_and_check(name, &PIDREF_MAKE_FROM_PID(pid), flags);
}
/*
* Return values:
*
* < 0 : wait_for_terminate_with_timeout() failed to get the state of the process, the process timed out, the process
* was terminated by a signal, or failed for an unknown reason.
*
* >=0 : The process terminated normally with no failures.
*
* Success is indicated by a return value of zero, a timeout is indicated by ETIMEDOUT, and all other child failure
* states are indicated by error is indicated by a non-zero value.
*
* This call assumes SIGCHLD has been blocked already, in particular before the child to wait for has been forked off
* to remain entirely race-free.
*/
int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout) {
sigset_t mask;
int r;
usec_t until;
assert_se(sigemptyset(&mask) == 0);
assert_se(sigaddset(&mask, SIGCHLD) == 0);
/* Drop into a sigtimewait-based timeout. Waiting for the
* pid to exit. */
until = usec_add(now(CLOCK_MONOTONIC), timeout);
for (;;) {
usec_t n;
siginfo_t status = {};
n = now(CLOCK_MONOTONIC);
if (n >= until)
break;
r = RET_NERRNO(sigtimedwait(&mask, NULL, TIMESPEC_STORE(until - n)));
/* Assuming we woke due to the child exiting. */
if (waitid(P_PID, pid, &status, WEXITED|WNOHANG) == 0) {
if (status.si_pid == pid) {
/* This is the correct child. */
if (status.si_code == CLD_EXITED)
return status.si_status == 0 ? 0 : -EPROTO;
else
return -EPROTO;
}
}
/* Not the child, check for errors and proceed appropriately */
if (r < 0) {
switch (r) {
case -EAGAIN:
/* Timed out, child is likely hung. */
return -ETIMEDOUT;
case -EINTR:
/* Received a different signal and should retry */
continue;
default:
/* Return any unexpected errors */
return r;
}
}
}
return -EPROTO;
}
void sigkill_wait(pid_t pid) {
assert(pid > 1);
src/basic/process-util.h
-2
View File
@@ -73,8 +73,6 @@ typedef enum WaitFlags {
int pidref_wait_for_terminate_and_check(const char *name, PidRef *pidref, WaitFlags flags);
int wait_for_terminate_and_check(const char *name, pid_t pid, WaitFlags flags);
int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout);
void sigkill_wait(pid_t pid);
void sigkill_waitp(pid_t *pid);
void sigterm_wait(pid_t pid);
src/core/socket.c
+2 -2
View File
@@ -1653,7 +1653,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.pid, WAIT_LOG_ABNORMAL);
r = pidref_wait_for_terminate_and_check("(sd-listen)", &pid, WAIT_LOG_ABNORMAL);
if (r < 0) {
safe_close(fd);
return r;
@@ -3181,7 +3181,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.pid, WAIT_LOG_ABNORMAL);
r = pidref_wait_for_terminate_and_check("(sd-accept)", &pid, WAIT_LOG_ABNORMAL);
if (r < 0) {
safe_close(cfd);
return r;
src/home/homed-home.c
+5 -2
View File
@@ -3299,11 +3299,14 @@ int home_wait_for_worker(Home *h) {
log_info("Worker process for home %s is still running while exiting. Waiting for it to finish.", h->user_name);
r = wait_for_terminate_with_timeout(h->worker_pid.pid, 30 * USEC_PER_SEC);
siginfo_t si;
r = pidref_wait_for_terminate_full(&h->worker_pid, 30 * USEC_PER_SEC, &si);
if (r == -ETIMEDOUT)
log_warning_errno(r, "Waiting for worker process for home %s timed out. Ignoring.", h->user_name);
else if (r < 0)
log_warning_errno(r, "Failed to wait for worker process for home %s. Ignoring.", h->user_name);
log_warning_errno(r, "Failed to wait for worker process for home %s, ignoring: %m", h->user_name);
else if (si.si_code != CLD_EXITED || si.si_status != 0)
log_warning("Worker process for home %s failed with non-zero exit status. Ignoring.", h->user_name);
(void) hashmap_remove_value(h->manager->homes_by_worker_pid, &h->worker_pid, h);
pidref_done(&h->worker_pid);
src/shutdown/shutdown.c
+1 -3
View File
@@ -228,8 +228,6 @@ int sync_with_progress(int fd) {
_cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
int r;
BLOCK_SIGNALS(SIGCHLD);
/* Due to the possibility of the sync operation hanging, we fork a child process and monitor
* the progress. If the timeout lapses, the assumption is that the particular sync stalled. */
@@ -252,7 +250,7 @@ int sync_with_progress(int fd) {
* SYNC_PROGRESS_ATTEMPTS lapse without progress being made,
* we assume that the sync is stalled */
for (unsigned checks = 0; checks < SYNC_PROGRESS_ATTEMPTS; checks++) {
r = wait_for_terminate_with_timeout(pidref.pid, SYNC_TIMEOUT_USEC);
r = pidref_wait_for_terminate_full(&pidref, SYNC_TIMEOUT_USEC, /* ret= */ NULL);
if (r == 0)
/* Sync finished without error (sync() call itself does not return an error code) */
return 0;
src/shutdown/umount.c
+47 -31
View File
@@ -24,9 +24,9 @@
#include "mount-util.h"
#include "mountpoint-util.h"
#include "parse-util.h"
#include "pidref.h"
#include "process-util.h"
#include "random-util.h"
#include "signal-util.h"
#include "stat-util.h"
#include "string-util.h"
#include "umount.h"
@@ -250,11 +250,9 @@ static void log_umount_blockers(const char *mnt) {
static int remount_with_timeout(MountPoint *m, bool last_try) {
_cleanup_close_pair_ int pfd[2] = EBADF_PAIR;
_cleanup_(sigkill_nowaitp) pid_t pid = 0;
_cleanup_(pidref_done_sigkill_nowait) PidRef pidref = PIDREF_NULL;
int r;
BLOCK_SIGNALS(SIGCHLD);
assert(m);
r = pipe2(pfd, O_CLOEXEC|O_NONBLOCK);
@@ -263,10 +261,10 @@ static int remount_with_timeout(MountPoint *m, bool last_try) {
/* Due to the possibility of a remount operation hanging, we fork a child process and set a
* timeout. If the timeout lapses, the assumption is that the particular remount failed. */
r = safe_fork_full("(sd-remount)",
r = pidref_safe_fork_full("(sd-remount)",
NULL,
pfd, ELEMENTSOF(pfd),
FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_REOPEN_LOG, &pid);
FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_REOPEN_LOG, &pidref);
if (r < 0)
return r;
if (r == 0) {
@@ -287,29 +285,37 @@ static int remount_with_timeout(MountPoint *m, bool last_try) {
pfd[1] = safe_close(pfd[1]);
r = wait_for_terminate_with_timeout(pid, DEFAULT_TIMEOUT_USEC);
siginfo_t si;
r = pidref_wait_for_terminate_full(&pidref, DEFAULT_TIMEOUT_USEC, &si);
if (r == -ETIMEDOUT)
log_error_errno(r, "Remounting '%s' timed out, issuing SIGKILL to PID " PID_FMT ".", m->path, pid);
else if (r == -EPROTO) {
log_error_errno(r, "Remounting '%s' timed out, issuing SIGKILL to PID " PID_FMT ".", m->path, pidref.pid);
else if (r < 0)
log_error_errno(r, "Remounting '%s' failed unexpectedly, couldn't wait for child process " PID_FMT ": %m", m->path, pidref.pid);
else if (si.si_code != CLD_EXITED || si.si_status != 0) {
/* Try to read error code from child */
if (read(pfd[0], &r, sizeof(r)) == sizeof(r))
log_debug_errno(r, "Remounting '%s' failed abnormally, child process " PID_FMT " failed: %m", m->path, pid);
else
r = log_debug_errno(EPROTO, "Remounting '%s' failed abnormally, child process " PID_FMT " aborted or exited non-zero.", m->path, pid);
TAKE_PID(pid); /* child exited (just not as we expected) hence don't kill anymore */
} else if (r < 0)
log_error_errno(r, "Remounting '%s' failed unexpectedly, couldn't wait for child process " PID_FMT ": %m", m->path, pid);
r = read_errno(pfd[0]);
if (r == -EIO)
r = log_debug_errno(
SYNTHETIC_ERRNO(EPROTO),
"Remounting '%s' failed abnormally, child process " PID_FMT " aborted or exited non-zero.",
m->path, pidref.pid);
else if (r < 0)
log_debug_errno(
r,
"Remounting '%s' failed abnormally, child process " PID_FMT " failed: %m",
m->path, pidref.pid);
TAKE_PIDREF(pidref); /* child exited (just not as we expected) hence don't kill anymore */
}
return r;
}
static int umount_with_timeout(MountPoint *m, bool last_try) {
_cleanup_close_pair_ int pfd[2] = EBADF_PAIR;
_cleanup_(sigkill_nowaitp) pid_t pid = 0;
_cleanup_(pidref_done_sigkill_nowait) PidRef pidref = PIDREF_NULL;
int r;
BLOCK_SIGNALS(SIGCHLD);
assert(m);
r = pipe2(pfd, O_CLOEXEC|O_NONBLOCK);
@@ -318,10 +324,10 @@ static int umount_with_timeout(MountPoint *m, bool last_try) {
/* Due to the possibility of a umount operation hanging, we fork a child process and set a
* timeout. If the timeout lapses, the assumption is that the particular umount failed. */
r = safe_fork_full("(sd-umount)",
r = pidref_safe_fork_full("(sd-umount)",
NULL,
pfd, ELEMENTSOF(pfd),
FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_REOPEN_LOG, &pid);
FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_REOPEN_LOG, &pidref);
if (r < 0)
return r;
if (r == 0) {
@@ -349,18 +355,28 @@ static int umount_with_timeout(MountPoint *m, bool last_try) {
pfd[1] = safe_close(pfd[1]);
r = wait_for_terminate_with_timeout(pid, DEFAULT_TIMEOUT_USEC);
siginfo_t si;
r = pidref_wait_for_terminate_full(&pidref, DEFAULT_TIMEOUT_USEC, &si);
if (r == -ETIMEDOUT)
log_error_errno(r, "Unmounting '%s' timed out, issuing SIGKILL to PID " PID_FMT ".", m->path, pid);
else if (r == -EPROTO) {
log_error_errno(r, "Unmounting '%s' timed out, issuing SIGKILL to PID " PID_FMT ".", m->path, pidref.pid);
else if (r < 0)
log_error_errno(r, "Unmounting '%s' failed unexpectedly, couldn't wait for child process " PID_FMT ": %m", m->path, pidref.pid);
else if (si.si_code != CLD_EXITED || si.si_status != 0) {
/* Try to read error code from child */
if (read(pfd[0], &r, sizeof(r)) == sizeof(r))
log_debug_errno(r, "Unmounting '%s' failed abnormally, child process " PID_FMT " failed: %m", m->path, pid);
else
r = log_debug_errno(EPROTO, "Unmounting '%s' failed abnormally, child process " PID_FMT " aborted or exited non-zero.", m->path, pid);
TAKE_PID(pid); /* It died, but abnormally, no purpose in killing */
} else if (r < 0)
log_error_errno(r, "Unmounting '%s' failed unexpectedly, couldn't wait for child process " PID_FMT ": %m", m->path, pid);
r = read_errno(pfd[0]);
if (r == -EIO)
r = log_debug_errno(
SYNTHETIC_ERRNO(EPROTO),
"Unmounting '%s' failed abnormally, child process " PID_FMT " aborted or exited non-zero.",
m->path, pidref.pid);
else if (r < 0)
log_debug_errno(
r,
"Unmounting '%s' failed abnormally, child process " PID_FMT " failed: %m",
m->path, pidref.pid);
TAKE_PIDREF(pidref); /* It died, but abnormally, no purpose in killing */
}
return r;
}
src/test/test-pidref.c
+19
View File
@@ -6,6 +6,7 @@
#include "process-util.h"
#include "stdio-util.h"
#include "tests.h"
#include "time-util.h"
TEST(pidref_is_set) {
ASSERT_FALSE(pidref_is_set(NULL));
@@ -246,4 +247,22 @@ TEST(pidref_is_remote) {
ASSERT_ERROR(pidref_verify(&p), EREMOTE);
}
TEST(pidref_wait_for_terminate_timeout) {
_cleanup_(pidref_done_sigkill_wait) PidRef pidref = PIDREF_NULL;
siginfo_t si;
/* Test successful termination within timeout */
ASSERT_OK(pidref_safe_fork("(test-pidref-wait-timeout)", FORK_DEATHSIG_SIGKILL|FORK_FREEZE, &pidref));
assert_se(pidref_kill(&pidref, SIGKILL) >= 0);
ASSERT_OK(pidref_wait_for_terminate_full(&pidref, 5 * USEC_PER_SEC, &si));
ASSERT_EQ(si.si_signo, SIGCHLD);
pidref_done(&pidref);
/* Test timeout when process doesn't terminate */
ASSERT_OK(pidref_safe_fork("(test-pidref-wait-timeout-expired)", FORK_DEATHSIG_SIGKILL|FORK_FREEZE, &pidref));
ASSERT_ERROR(pidref_wait_for_terminate_full(&pidref, 100 * USEC_PER_MSEC, NULL), ETIMEDOUT);
}
DEFINE_TEST_MAIN(LOG_DEBUG);