master a81669c: Fix hang when deleting a pipe process

[Eli Zaretskii]

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

    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)
---
 src/w32.c     | 28 +++++++++++++++++++++++-----
 src/w32.h     |  1 +
 src/w32proc.c | 21 +++++++++++++++------
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/src/w32.c b/src/w32.c
index 2b2f8aa..1de148f 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 b31d666..bb3ec40 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 360f45e..bfe720e 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;
 }