From d16d463e0d9fa8dd5f7af141fa85d2c5d6e06b4a Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sun, 17 Jun 2018 22:26:48 -0500 Subject: [PATCH] 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. --- src/postfork.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/postfork.cpp b/src/postfork.cpp index 2751411d2..93281b28a 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -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();