[Emacs-diffs] /srv/bzr/emacs/trunk r111311: Improve handling of subprocess shutdown on MS-Windows.

[Eli Zaretskii]

------------------------------------------------------------
revno: 111311
committer: Eli Zaretskii <address@hidden>
branch nick: trunk
timestamp: Sun 2012-12-23 19:06:58 +0200
message:
  Improve handling of subprocess shutdown on MS-Windows.
  
   src/w32proc.c (reader_thread): Do not index fd_info[] with negative
   values.
   (reader_thread): Exit when cp->status becomes STATUS_READ_ERROR
   after WaitForSingleObject returns normally.  This expedites reader
   thread shutdown when delete_child triggers it.
   (reap_subprocess): More accurate commentary for why we call
   delete_child only when cp->fd is negative.
   src/w32.c (sys_close): Do not call delete_child on a subprocess
   whose handle is not yet closed.  Instead, set its file descriptor
   to a negative value, so that reap_subprocess will call
   delete_child on that subprocess when its SIGCHLD arrives.  This
   avoids closing handles used for communications between sys_select
   and reader_thread, which doesn't give sys_select a chance to
   notice that the process exited and invoke the SIGCHLD handler for
   it.
modified:
  src/ChangeLog
  src/w32.c
  src/w32proc.c

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2012-12-23 12:35:37 +0000
+++ b/src/ChangeLog     2012-12-23 17:06:58 +0000
@@ -1,3 +1,22 @@
+2012-12-23  Eli Zaretskii  <address@hidden>
+
+       * w32proc.c (reader_thread): Do not index fd_info[] with negative
+       values.
+       (reader_thread): Exit when cp->status becomes STATUS_READ_ERROR
+       after WaitForSingleObject returns normally.  This expedites reader
+       thread shutdown when delete_child triggers it.
+       (reap_subprocess): More accurate commentary for why we call
+       delete_child only when cp->fd is negative.
+
+       * w32.c (sys_close): Do not call delete_child on a subprocess
+       whose handle is not yet closed.  Instead, set its file descriptor
+       to a negative value, so that reap_subprocess will call
+       delete_child on that subprocess when its SIGCHLD arrives.  This
+       avoids closing handles used for communications between sys_select
+       and reader_thread, which doesn't give sys_select a chance to
+       notice that the process exited and invoke the SIGCHLD handler for
+       it.
+
 2012-12-23  Jan Djärv  <address@hidden>
 
        * nsfns.m (Fns_do_applescript): Run event loop until script has

=== modified file 'src/w32.c'
--- a/src/w32.c 2012-12-21 19:32:43 +0000
+++ b/src/w32.c 2012-12-23 17:06:58 +0000
@@ -6401,7 +6401,21 @@
 
                  winsock_inuse--; /* count open sockets */
                }
-             delete_child (cp);
+             /* If the process handle is NULL, it's either a socket
+                or serial connection, or a subprocess that was
+                already reaped by reap_subprocess, but whose
+                resources were not yet freed, because its output was
+                not fully read yet by the time it was reaped.  (This
+                usually happens with async subprocesses whose output
+                is being read by Emacs.)  Otherwise, this process was
+                not reaped yet, so we set its FD to a negative value
+                to make sure sys_select will eventually get to
+                calling the SIGCHLD handler for it, which will then
+                invoke waitpid and reap_subprocess.  */
+             if (cp->procinfo.hProcess == NULL)
+               delete_child (cp);
+             else
+               cp->fd = -1;
            }
        }
     }

=== modified file 'src/w32proc.c'
--- a/src/w32proc.c     2012-12-21 19:32:43 +0000
+++ b/src/w32proc.c     2012-12-23 17:06:58 +0000
@@ -965,7 +965,7 @@
     {
       int rc;
 
-      if (fd_info[cp->fd].flags & FILE_LISTEN)
+      if (cp->fd >= 0 && fd_info[cp->fd].flags & FILE_LISTEN)
        rc = _sys_wait_accept (cp->fd);
       else
        rc = _sys_read_ahead (cp->fd);
@@ -993,6 +993,8 @@
                     "%lu for fd %ld\n", GetLastError (), cp->fd));
          break;
         }
+      if (cp->status == STATUS_READ_ERROR)
+       break;
     }
   return 0;
 }
@@ -1163,11 +1165,11 @@
       cp->procinfo.hThread = NULL;
     }
 
-  /* For asynchronous children, the child_proc resources will be freed
-     when the last pipe read descriptor is closed; for synchronous
-     children, we must explicitly free the resources now because
-     register_child has not been called. */
-  if (cp->fd == -1)
+  /* If cp->fd was not closed yet, we might be still reading the
+     process output, so don't free its resources just yet.  The call
+     to delete_child on behalf of this subprocess will be made by
+     sys_read when the subprocess output is fully read.  */
+  if (cp->fd < 0)
     delete_child (cp);
 }