Silence EACCES errors upon setpgid after posix_spawn()

We've tried numerous approaches to mitigate the race condition between
`posix_spawn` and the `setpgid` call, but unfortunately due to the flags
we pass to `posix_spawn`, it (rarely? never?) results in `vfork()` being
used, which means it is never executed atomically. Since it is executed
out-of-band, we must manually call `setpgid` in case `posix_spawn`
hasn't gotten around to doing that yet, but in the event that it has, an
EACCES error can be returned.

Closes #4884. Closes #4715. See also #4778.
This commit is contained in:
Mahmoud Al-Qudsi
2018-06-17 22:26:48 -05:00
parent 072974ec5c
commit d16d463e0d

View File

@@ -106,15 +106,25 @@ bool child_set_group(job_t *j, process_t *p) {
/// if it's to run in the foreground.
bool set_child_group(job_t *j, pid_t child_pid) {
if (j->get_flag(JOB_CONTROL)) {
assert (j->pgid != -2 && "set_child_group called with JOB_CONTROL before job pgid determined!");
assert (j->pgid != -2
&& "set_child_group called with JOB_CONTROL before job pgid determined!");
// The parent sets the child's group. This incurs the well-known unavoidable race with the
// child exiting, so ignore ESRCH and EPERM (in case the pid was recycled).
// Additionally ignoring EACCES. See #4715 and #4884.
if (setpgid(child_pid, j->pgid) < 0) {
if (errno != ESRCH && errno != EPERM) {
if (errno != ESRCH && errno != EPERM && errno != EACCES) {
safe_perror("setpgid");
return false;
}
else {
// Just in case it's ever not right to ignore the setpgid call, (i.e. if this
// ever leads to a terminal hang due if both this setpgid call AND posix_spawn's
// internal setpgid calls failed), write to the debug log so a future developer
// doesn't go crazy trying to track this down.
debug(2, "Error %d while calling setpgid for child %d (probably harmless)",
errno, child_pid);
}
}
} else {
j->pgid = getpgrp();