Skip to content

Commit

Permalink
manager: always reap first the process for which we already got SIGCHLD
Browse files Browse the repository at this point in the history
If we get a SIGCHLD we enable and eventually dispatch
sigchld_event_source where we actually reap the process. We received
SIGCHLD for the specific PID so wait for that process first.

Motivation to do this is to prevent problem due to our state machine for
mount units relying on the fact that we always dispatch mountinfo
notifications before dispatching sigchld handler for the
mount. Previously, this was racy because we might have called
manager_dispatch_sigchld() for completely unrelated process but we would
actually reap the mount process which completed in the meantime. sigchld
handler for the mount unit would then fail the mount unit because we
haven't dispatched mountinfo notification yet.

event| mount         kernel              PID 1
------------------------------------------------------------------------
1    |                                   forks off mount as PID x
------------------------------------------------------------------------
2    |                                   receives SIGCHLD for PID y
------------------------------------------------------------------------
3    |                                   enables sigchld_event_source
------------------------------------------------------------------------
4    |                                   dispatches sigchld_event_source
------------------------------------------------------------------------
5    | mount()       mountinfo_notif
------------------------------------------------------------------------
6    | exit()
------------------------------------------------------------------------
7    |                                   calls waitid() with P_ALL
------------------------------------------------------------------------
8    |                                   calls sigchld_handler for mount
------------------------------------------------------------------------
9    |                                   fails the mount unit since
     |                                   mountinfo_notif wasn't
     |                                   processed yet
------------------------------------------------------------------------

Fixes systemd#10872
  • Loading branch information
msekletar committed Jul 3, 2019
1 parent afb30c9 commit eb653ed
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
29 changes: 27 additions & 2 deletions src/core/manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -2470,6 +2470,27 @@ static void manager_invoke_sigchld_event(
UNIT_VTABLE(u)->sigchld_event(u, si->si_pid, si->si_code, si->si_status);
}

static int manager_waitid(Manager *m, siginfo_t *si) {
id_t pid, type;

assert(m);
assert(si);

/* We may have cached PID of the process for which we got SIGCHLD already but haven't waited for yet,
* if so then we wait for it, otherwise wait for any process. */
if (m->sigchld_pid) {
pid = m->sigchld_pid;
type = P_PID;

m->sigchld_pid = 0;
} else {
pid = 0;
type = P_ALL;
}

return waitid(type, pid, si, WEXITED|WNOHANG|WNOWAIT);
}

static int manager_dispatch_sigchld(sd_event_source *source, void *userdata) {
Manager *m = userdata;
siginfo_t si = {};
Expand All @@ -2481,8 +2502,8 @@ static int manager_dispatch_sigchld(sd_event_source *source, void *userdata) {
/* First we call waitid() for a PID and do not reap the zombie. That way we can still access /proc/$PID for it
* while it is a zombie. */

if (waitid(P_ALL, 0, &si, WEXITED|WNOHANG|WNOWAIT) < 0) {

r = manager_waitid(m, &si);
if (r < 0) {
if (errno != ECHILD)
log_error_errno(errno, "Failed to peek for child with waitid(), ignoring: %m");

Expand Down Expand Up @@ -2626,6 +2647,10 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
if (r < 0)
log_warning_errno(r, "Failed to enable SIGCHLD event source, ignoring: %m");

/* Cache the PID of the process so that we wait for it
* once we dispatch the event source. */
m->sigchld_pid = sfsi.ssi_pid;

break;

case SIGTERM:
Expand Down
1 change: 1 addition & 0 deletions src/core/manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ struct Manager {
sd_event_source *signal_event_source;

sd_event_source *sigchld_event_source;
pid_t sigchld_pid;

int time_change_fd;
sd_event_source *time_change_event_source;
Expand Down

0 comments on commit eb653ed

Please sign in to comment.