From d1913f0df0e5d05fa553b6d482f1cb71261f22ba Mon Sep 17 00:00:00 2001
From: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Wed, 2 Jan 2019 00:12:07 -0600
Subject: [PATCH] Add workaround for Cygwin process management and job control
 bugs

We cannot wait by pgroup under Cygwin for unknown reasons. Always
wait on jobs by individual processes. See code for more information.
---
 cmake/ConfigureChecks.cmake |  5 +++++
 config_cmake.h.in           |  3 +++
 src/common.h                |  9 +++++++++
 src/proc.cpp                | 24 +++++++++++++++++++++---
 4 files changed, 38 insertions(+), 3 deletions(-)

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