master 637436970f: Fix leaking of file descriptors due to pipe processes on MS-Windows

[Eli Zaretskii]

branch: master
commit 637436970f34f860d50f73a514b3bafd0c5cace7
Author: Eli Zaretskii <eliz@gnu.org>
Commit: Eli Zaretskii <eliz@gnu.org>

    Fix leaking of file descriptors due to pipe processes on MS-Windows
    
    * src/w32proc.c (reader_thread): Wait for 'sys_close' to finish
    processing the pipe read descriptor, before trying to close it.
    
    * src/w32.c (sys_close): Attempt to detect when the reader thread
    already exited, so that it would be possible to close descriptors
    open by pipe processes for reading from the pipe.  (Bug#56606)
---
 src/w32.c     | 23 ++++++++++++++++++-----
 src/w32proc.c | 17 ++++++++++++++---
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/src/w32.c b/src/w32.c
index e4c6d00766..cbcfcdd4f6 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -8573,6 +8573,7 @@ int
 sys_close (int fd)
 {
   int rc = -1;
+  bool reader_thread_exited = false;
 
   if (fd < 0)
     {
@@ -8583,6 +8584,13 @@ sys_close (int fd)
   if (fd < MAXDESC && fd_info[fd].cp)
     {
       child_process * cp = fd_info[fd].cp;
+      DWORD thrd_status = STILL_ACTIVE;
+
+      /* Thread handle will be NULL if we already called delete_child.  */
+      if (cp->thrd != NULL
+         && GetExitCodeThread (cp->thrd, &thrd_status)
+         && thrd_status != STILL_ACTIVE)
+       reader_thread_exited = true;
 
       fd_info[fd].cp = NULL;
 
@@ -8633,7 +8641,11 @@ sys_close (int fd)
      because socket handles are fully fledged kernel handles. */
   if (fd < MAXDESC)
     {
-      if ((fd_info[fd].flags & FILE_DONT_CLOSE) == 0)
+      if ((fd_info[fd].flags & FILE_DONT_CLOSE) == 0
+         /* If the reader thread already exited, close the descriptor,
+            since otherwise no one will close it, and we will be
+            leaking descriptors.  */
+         || reader_thread_exited)
        {
          fd_info[fd].flags = 0;
          rc = _close (fd);
@@ -8641,10 +8653,11 @@ sys_close (int fd)
       else
        {
          /* We don't close here descriptors open by pipe processes
-            for reading from the pipe, because the reader thread
-            might be stuck in _sys_read_ahead, and then we will hang
-            here.  If the reader thread exits normally, it will close
-            the descriptor; otherwise we will leave a zombie thread
+            for reading from the pipe when the reader thread might
+            still be running, since that thread might be stuck in
+            _sys_read_ahead, and then we will hang here.  If the
+            reader thread exits normally, it will close the
+            descriptor; otherwise we will leave a zombie thread
             hanging around.  */
          rc = 0;
          /* Leave the flag set for the reader thread to close the
diff --git a/src/w32proc.c b/src/w32proc.c
index 7acfba64d7..f771ebc851 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -1287,10 +1287,21 @@ reader_thread (void *arg)
     }
   /* If this thread was reading from a pipe process, close the
      descriptor used for reading, as sys_close doesn't in that case.  */
-  if (fd_info[fd].flags == FILE_DONT_CLOSE)
+  if ((fd_info[fd].flags & FILE_DONT_CLOSE) == FILE_DONT_CLOSE)
     {
-      fd_info[fd].flags = 0;
-      _close (fd);
+      int i;
+      /* If w32.c:sys_close is still processing this descriptor, wait
+        for a while for it to finish.  */
+      for (i = 0; i < 5; i++)
+       {
+         if (fd_info[fd].flags == FILE_DONT_CLOSE)
+           {
+             fd_info[fd].flags = 0;
+             _close (fd);
+             break;
+           }
+         Sleep (5);
+       }
     }
   return 0;
 }