From: Eli Zaretskii <eliz@gnu.org>
Date: Sat, 11 Dec 2021 18:15:53 +0000 (+0200)
Subject: Fix hang when deleting a pipe process
X-Git-Tag: emacs-29.0.90~3600^2~5
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=a81669c69fda1a2d0d4238b8440145fb2aeb959f;p=emacs.git

Fix hang when deleting a pipe process

* src/w32.h (FILE_DONT_CLOSE): New flag.
* src/w32.c (sys_close): Don't close descriptors used to read from
the pipe process.  Leave the FILE_DONT_CLOSE flag set in the
descriptor's info.
(register_aux_fd): Set the FILE_DONT_CLOSE flag in the
descriptor's info.
* src/w32proc.c (reader_thread): When exiting normally, close the
file descriptor used to read from a pipe process.  (Bug#52414)
---

diff --git a/src/w32.c b/src/w32.c
index 2b2f8aadf6b..1de148f0343 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -8548,7 +8548,7 @@ fcntl (int s, int cmd, int options)
 int
 sys_close (int fd)
 {
-  int rc;
+  int rc = -1;
 
   if (fd < 0)
     {
@@ -8603,14 +8603,31 @@ sys_close (int fd)
 	}
     }
 
-  if (fd >= 0 && fd < MAXDESC)
-    fd_info[fd].flags = 0;
-
   /* Note that sockets do not need special treatment here (at least on
      NT and Windows 95 using the standard tcp/ip stacks) - it appears that
      closesocket is equivalent to CloseHandle, which is to be expected
      because socket handles are fully fledged kernel handles. */
-  rc = _close (fd);
+  if (fd < MAXDESC)
+    {
+      if ((fd_info[fd].flags & FILE_DONT_CLOSE) == 0)
+	{
+	  fd_info[fd].flags = 0;
+	  rc = _close (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
+	     hanging around.  */
+	  rc = 0;
+	  /* Leave the flag set for the reader thread to close the
+	     descriptor.  */
+	  fd_info[fd].flags = FILE_DONT_CLOSE;
+	}
+    }
 
   return rc;
 }
@@ -10898,6 +10915,7 @@ register_aux_fd (int infd)
     }
   fd_info[ infd ].cp = cp;
   fd_info[ infd ].hnd = (HANDLE) _get_osfhandle (infd);
+  fd_info[ infd ].flags |= FILE_DONT_CLOSE;
 }
 
 #ifdef HAVE_GNUTLS
diff --git a/src/w32.h b/src/w32.h
index b31d66646c9..bb3ec40324a 100644
--- a/src/w32.h
+++ b/src/w32.h
@@ -135,6 +135,7 @@ extern filedesc fd_info [ MAXDESC ];
 #define FILE_SOCKET             0x0200
 #define FILE_NDELAY             0x0400
 #define FILE_SERIAL             0x0800
+#define FILE_DONT_CLOSE         0x1000
 
 extern child_process * new_child (void);
 extern void delete_child (child_process *cp);
diff --git a/src/w32proc.c b/src/w32proc.c
index 360f45e9e11..bfe720eb623 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -1206,6 +1206,7 @@ static DWORD WINAPI
 reader_thread (void *arg)
 {
   child_process *cp;
+  int fd;
 
   /* Our identity */
   cp = (child_process *)arg;
@@ -1220,12 +1221,13 @@ reader_thread (void *arg)
     {
       int rc;
 
-      if (cp->fd >= 0 && (fd_info[cp->fd].flags & FILE_CONNECT) != 0)
-	rc = _sys_wait_connect (cp->fd);
-      else if (cp->fd >= 0 && (fd_info[cp->fd].flags & FILE_LISTEN) != 0)
-	rc = _sys_wait_accept (cp->fd);
+      fd = cp->fd;
+      if (fd >= 0 && (fd_info[fd].flags & FILE_CONNECT) != 0)
+	rc = _sys_wait_connect (fd);
+      else if (fd >= 0 && (fd_info[fd].flags & FILE_LISTEN) != 0)
+	rc = _sys_wait_accept (fd);
       else
-	rc = _sys_read_ahead (cp->fd);
+	rc = _sys_read_ahead (fd);
 
       /* Don't bother waiting for the event if we already have been
 	 told to exit by delete_child.  */
@@ -1238,7 +1240,7 @@ reader_thread (void *arg)
         {
 	  DebPrint (("reader_thread.SetEvent(0x%x) failed with %lu for fd %ld (PID %d)\n",
 		     (DWORD_PTR)cp->char_avail, GetLastError (),
-		     cp->fd, cp->pid));
+		     fd, cp->pid));
 	  return 1;
 	}
 
@@ -1266,6 +1268,13 @@ reader_thread (void *arg)
       if (cp->status == STATUS_READ_ERROR)
 	break;
     }
+  /* 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)
+    {
+      fd_info[fd].flags = 0;
+      _close (fd);
+    }
   return 0;
 }