diff --git a/cmake/ConfigureChecks.cmake b/cmake/ConfigureChecks.cmake index 6b4f0b99f..265a96127 100644 --- a/cmake/ConfigureChecks.cmake +++ b/cmake/ConfigureChecks.cmake @@ -19,6 +19,11 @@ if (CMAKE_HOST_SYSTEM_VERSION MATCHES ".*-Microsoft") SET(WSL 1) endif() +# Detect Cygwin. +if (CMAKE_SYSTEM_NAME MATCHES "CYGWIN_NT.*") + SET(CYGWIN 1) +endif() + # Set up the config.h file. SET(PACKAGE_NAME "fish") SET(PACKAGE_TARNAME "fish") diff --git a/config_cmake.h.in b/config_cmake.h.in index 47cfcc5e2..162f76de0 100644 --- a/config_cmake.h.in +++ b/config_cmake.h.in @@ -4,6 +4,9 @@ /* Define to 1 if compiled on WSL */ #cmakedefine WSL 1 +/* Define to 1 if complied under Cygwin */ +#cmakedefine CYGWIN 1 + /* Define to 1 if you have the `clock_gettime' function. */ #cmakedefine HAVE_CLOCK_GETTIME 1 diff --git a/src/common.h b/src/common.h index 889036841..dac5d9a6d 100644 --- a/src/common.h +++ b/src/common.h @@ -947,6 +947,15 @@ constexpr bool is_windows_subsystem_for_linux() { #endif } +/// Detect if we are running under Cygwin or Cgywin64 +constexpr bool is_cygwin() { +#ifdef CYGWIN + return true; +#else + return false; +#endif +} + extern "C" { __attribute__((noinline)) void debug_thread_error(void); } diff --git a/src/proc.cpp b/src/proc.cpp index 8072a64a7..69494c8dc 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -448,9 +448,27 @@ static bool process_mark_finished_children(bool block_on_fg) { options &= ~WNOHANG; } - // If the pgid is 0, we need to wait by process because that's invalid. - // This happens in firejail for reasons not entirely clear to me. - bool wait_by_process = !j->job_chain_is_fully_constructed() || j->pgid == 0; + // Child jobs (produced via execution of functions) share job ids with their not-yet- + // fully-constructed parent jobs, so we have to wait on these by individual process id + // and not by the shared pgroup. End result is the same, but it just makes more calls + // to the kernel. + bool wait_by_process = !j->job_chain_is_fully_constructed(); + + // Firejail can result in jobs with pgroup 0, in which case we cannot wait by + // job id. See discussion in #5295. + if (j->pgid == 0) { + wait_by_process = true; + } + + // Cygwin does some voodoo with regards to process management that I do not understand, but + // long story short, we cannot reap processes by their pgroup. The way child processes are + // launched under Cygwin is... weird, and outwardly they do not appear to retain information + // about their parent process when viewed in Task Manager. Waiting on processes by their + // pgroup results in never reaping any, so we just wait on them by process id instead. + if (is_cygwin()) { + wait_by_process = true; + } + // When waiting on processes individually in a pipeline, we need to enumerate in reverse // order so that the first process we actually wait on (i.e. ~WNOHANG) is the last process // in the IO chain, because that's the one that controls the lifetime of the foreground job