From: Eli Zaretskii <eliz@gnu.org>
Date: Sun, 17 Jul 2022 12:44:50 +0000 (+0300)
Subject: Fix leaking of file descriptors due to pipe processes on MS-Windows
X-Git-Tag: emacs-29.0.90~1447^2~880
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=637436970f34f860d50f73a514b3bafd0c5cace7;p=emacs.git

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)
---

diff --git a/src/w32.c b/src/w32.c
index e4c6d007661..cbcfcdd4f6d 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 7acfba64d70..f771ebc8511 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;
 }