Re: Running two processes rapidly makes Emacs eat 100% CPU on w32

[Slawomir Nowaczyk]

On Fri, 13 Oct 2006 17:50:35 +0200
Eli Zaretskii <address@hidden> wrote:

#> >            DebPrint (("select waiting on child %d fd %d\n",
#> >                       cp-child_procs, i));
#> > keeps printing "select waiting on child 0 fd 3" (thousands of times per
#> > second, every time sys_select is called.
#> 
#> Looks like somehow Emacs doesn't pay attention that the process
#> exited, and keeps trying to read its pipe. Do you agree with this
#> conclusion?

I have finally found some time to dig deeper into this issue. I still
don't quite understand the code, so take all that follows with a (big)
grain of salt...

For those who forgot, evaluating the following code makes Emacs eat 100%
CPU on my Windows machine: (progn (start-process "" nil "ls") (call-process 
"ls"))

I have tracked the problem in sys_select to the fact that
cp->procinfo.hProcess was being set to NULL prematurely... this caused
cp->the "if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess" test on line
cp->1202 to fail, in effect preventing Emacs from calling SIGCHLD
cp->properly.

Following this discovery, I have verified that hProcess is being set to
NULL by reap_subprocess, which get called from sys_wait.

Some poking around in sys_wait made me aware that line 508
"active = WaitForMultipleObjects (nh, wait_hnd, FALSE, 1000);"
is returning the "wrong" process... it was returning one created by
start-process, while it should have been returning the one created by
call-process (the distinction is important because this process was to
be reap_subprocess'ed immediately -- which is OK for processes from
call-process, but wrong for ones from start-process).

This has lead me to a code some 20 lines above, which actually produces
a list of processes to be waited for. It gathers all children which
fulfill the condition "if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess)".

It seems that this condition is too weak. I am not sure what should it
be, but sys_select in line 1206 uses, in similar circumstances, the
following one:
    if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess
        && (cp->fd < 0
            || (fd_info[cp->fd].flags & FILE_SEND_SIGCHLD) == 0
            || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0))

The "(fd_info[cp->fd].flags & FILE_SEND_SIGCHLD) == 0" part is clearly
unsuitable for sys_wait, but the rest gives good results for me.

So, the following patch fixes the problem for me, but I have no way of
knowing if it really is a correct solution. I have been running with it
for a couple of days now and haven't notice anything wrong, though. I
have also verified that both processes are correctly reap_subprocess'ed
in the (progn (start-process "" nil "ls") (call-process "ls")) case.

**********************************************************************

--- m:/EmacsCVS/EmacsCVS/src/w32proc_orig.c     2006-09-26 20:28:27.518832000 
+0200
+++ m:/EmacsCVS/EmacsCVS/src/w32proc.c  2006-10-27 00:12:26.527475200 +0200
@@ -486,7 +486,8 @@
     {
       for (cp = child_procs+(child_proc_count-1); cp >= child_procs; cp--)
        /* some child_procs might be sockets; ignore them */
-       if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess)
+       if (CHILD_ACTIVE (cp) && cp->procinfo.hProcess
+      && (cp->fd < 0 || (fd_info[cp->fd].flags & FILE_AT_EOF) != 0))
          {
            wait_hnd[nh] = cp->procinfo.hProcess;
            cps[nh] = cp;

**********************************************************************

-- 
 Best wishes,
   Slawomir Nowaczyk
     ( address@hidden )

The best performance improvement is the transition
  from the nonworking state to the working state.