[andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Richard Stallman]

Would someone please DTRT, then ack?

------- Start of forwarded message -------
X-Spam-Status: No, score=0.0 required=5.0 tests=UNPARSEABLE_RELAY 
	autolearn=failed version=3.1.0
content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Date: Tue, 14 Aug 2007 17:07:56 +0200
Thread-Topic: Emacs 22.1 hung after delete-process 
Thread-Index: AcfehOP/Lb9ZX4TITIWgROp4XnRGAQ==
From: "Maguire, Andrew \(GE Infra, Energy\)" <andrew.maguire@ge.com>
To: <bug-gnu-emacs@gnu.org>
Subject: Emacs 22.1 hung after delete-process 

Using delete-process to kill a subprocess causes Emacs to hang
irretrievably if the sub-process does not immediately exit.

E.g. Using perl.exe (5.8.6) and controlling the HUP signal:

(setq perl-process
      (start-process "perl" "*perl*"
		     "perl.exe"
	   	     "-e"
	       	     "$|=1; print 'starting... '; $SIG{'HUP'} = sub { print 'killed!'; die; }; sleep(20);"))

(delete-process perl-process)

Windows may popup a process killed dialog, select "End now".
Emacs is now completely hung :-(

I have only tried this on Windows. Emacs 21.2 does not have this problem.
kill-process and quit-process are fine and do not hang Emacs.

Thanks,
Andrew

Ps. Sorry if this email is repeated, I got a bounced return.

In GNU Emacs 22.1.1 (i386-mingw-nt5.1.2600)
 of 2007-06-02 on RELEASE
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4) --cflags -Ic:/gnuwin32/include'

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: ENU
  locale-coding-system: cp1252
  default-enable-multibyte-characters: t

Major mode: C/l

Minor modes in effect:
  shell-dirtrack-mode: t
  show-paren-mode: t
  encoded-kbd-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: identity
  abbrev-mode: t

Recent input:
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <help-echo> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <help-echo> <mouse-1> 
<mouse-1> <help-echo> <mouse-1> <mouse-1> <help-echo> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <help-echo> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<help-echo> <help-echo> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <help-echo> <down-mouse-1> <help-echo> 
<drag-mouse-1> <help-echo> <mouse-1> <mouse-1> <down-mouse-1> 
<mouse-1> C-s C-w <C-home> C-s C-s C-s C-s C-s C-s 
<down-mouse-1> <mouse-1> C-s C-w C-w C-w C-s C-s C-s 
C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s C-s <down-mouse-1> 
<mouse-1> <prior> <prior> <prior> <prior> <prior> <next> 
<prior> <prior> <next> <next> <prior> <prior> <next> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> 
<up> <up> <up> C-s e n d i f C-s C-s C-s C-s C-r C-r 
C-r C-r C-s C-s C-s C-s C-s C-s C-s <down> C-s C-s 
C-r C-r <down> C-s M-p C-s C-s C-s <down-mouse-1> <mouse-1> 
C-s C-w C-w C-w C-s C-s C-s C-s C-s C-s C-s C-s <prior> 
<prior> <prior> <prior> <next> <next> <next> <next> 
<next> C-s C-s C-s <help-echo> <down-mouse-1> <mouse-1> 
<double-down-mouse-1> <mouse-movement> <mouse-movement> 
<double-drag-mouse-1> <help-echo> <switch-frame> <help-echo> 
<down-mouse-1> <mouse-1> <help-echo> <help-echo> <help-echo> 
<help-echo> <help-echo> <help-echo> <menu-bar> <help-menu> 
<report-emacs-bug>

Recent messages:
Quit
Mark saved where search started [4 times]
Region 60 in buffer A is empty [2 times]
Refining difference region 61 ...
Region 60 in buffer A is empty [2 times]
To drag modelines or buffers up and down, use the middle button.
Mark saved where search started
Mark set
Mark saved where search started [7 times]
Loading emacsbug...done


_______________________________________________
bug-gnu-emacs mailing list
bug-gnu-emacs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnu-emacs
------- End of forwarded message -------

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Glenn Morris]

Richard Stallman wrote:

> Would someone please DTRT, then ack?

Can't reproduce this one on GNU/Linux either. Though I'm not sure I
expect to, given that AFAICS, delete-process sends the (untrappable)
SIGKILL, and the perl stuff below is trapping SIGHUP.

Or is the issue that trying to trap SIGHUP somehow creates an hung
perl process on Windows? Is delete-process supposed to be graceful in
such cases?

> From: "Maguire, Andrew \(GE Infra, Energy\)" <andrew.maguire@ge.com>
> Subject: Emacs 22.1 hung after delete-process 
> To: <bug-gnu-emacs@gnu.org>
> Date: Tue, 14 Aug 2007 17:07:56 +0200
[...]
> Using delete-process to kill a subprocess causes Emacs to hang
> irretrievably if the sub-process does not immediately exit.
>
> E.g. Using perl.exe (5.8.6) and controlling the HUP signal:
>
> (setq perl-process
>       (start-process "perl" "*perl*"
> 		     "perl.exe"
> 	   	     "-e"
> 	       	     "$|=1; print 'starting... '; $SIG{'HUP'} = sub { print 'killed!'; die; }; sleep(20);"))
>
> (delete-process perl-process)
>
> Windows may popup a process killed dialog, select "End now".
> Emacs is now completely hung :-(

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

Hi,

On 8/16/07, Glenn Morris <rgm@gnu.org> wrote:
> Richard Stallman wrote:
>
> > Would someone please DTRT, then ack?

I can reproduce this on Emacs from CVS HEAD on XP using MinGW build
with latest GCC 4. series.
However, I cannot reproduce with the VS.NET (MS Visual Studio 2003) build.
I am building with MinGW with debug enabled and will attempt debugging
and post my findings.

-dky

-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Jason Rumney]

Glenn Morris wrote:
> Or is the issue that trying to trap SIGHUP somehow creates an hung
> perl process on Windows? Is delete-process supposed to be graceful in
> such cases?
>   

It seems to. Windows does not have real signals, so both perl and Emacs
are translating those to whatever the respective developers thought was
the closest equivalent in Windows. When Emacs tries to delete the
process, a Windows dialog pops up saying that the process is not
stopping, and asking if you'd like to forcibly kill it. I got as far
last night as tracking it down to the status_message call on line 6805
of process.c. A breakpoint after that line was never reached, but a
breakpoint on that line was, and if you killed the process before
continuing, Emacs would not hang.

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

Hi,

On 8/16/07, Jason Rumney <jasonr@gnu.org> wrote:
> Glenn Morris wrote:
> > Or is the issue that trying to trap SIGHUP somehow creates an hung
> > perl process on Windows? Is delete-process supposed to be graceful in
> > such cases?
> >
>
> It seems to. Windows does not have real signals, so both perl and Emacs
> are translating those to whatever the respective developers thought was
> the closest equivalent in Windows. When Emacs tries to delete

But, how is the VS.NET build able to handle this? I was not able to
reproduce in the VS.NET build. Could it be any way related to having
PERL built using MSVC (Visual studio compiler) and Emacs built using
GCC (MinGW) compiler?

-dky

-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

Hi,
 I have put it a fix that seems to solve this problem (observed on my
box). I find the issue in using SetEvent() and not waiting long enough
before calling TerminateThread(). Either we could remove the call the
TerminateThread() or increase the delay (that I have done) or ideally
use a critical section (add it in the child_process structure).

-dky


E:\cache\build\emacs\src
[dky]hg cdiff w32proc.c
*** emacs.58583440b48b\src\w32proc.c    2007-08-16 19:11:29.218750000 +0530
--- E:\cache\build\emacs\src\w32proc.c  2007-08-16 19:02:44.828125000 +0530
*************** delete_child (child_process *cp)
*** 216,222 ****
          /* let the thread exit cleanly if possible */
          cp->status = STATUS_READ_ERROR;
          SetEvent (cp->char_consumed);
!         if (WaitForSingleObject (cp->thrd, 1000) != WAIT_OBJECT_0)
            {
              DebPrint (("delete_child.WaitForSingleObject (thread) failed "
                         "with %lu for fd %ld\n", GetLastError (), cp->fd));
--- 216,222 ----
          /* let the thread exit cleanly if possible */
          cp->status = STATUS_READ_ERROR;
          SetEvent (cp->char_consumed);
!         if (WaitForSingleObject (cp->thrd, INFINITE) != WAIT_OBJECT_0)
            {
              DebPrint (("delete_child.WaitForSingleObject (thread) failed "
                         "with %lu for fd %ld\n", GetLastError (), cp->fd));


-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

Hi,
 I am going through the reader_thread() in w32proc.c. There is a loop
in which _sys_read_ahead OR _sys_wait_accept is called. I am
suspecting the thread gets into a blocking read (for _sys_read_ahead).
 Could this be causing a hang as the reader_thread is trying to read
from a process that has been killed?
 Making _sys_read_ahead call ReadFile() with OVERLAPPED structure
could allow us implement a cancel able IO. If the process is killed,
we can set an event which will make the ReadFile() return. So, the
thread calling reader_thread() can gracefully exit instead of calling
a TerminateThread() (which ideally must never be called on w32).

I need some guidance, I can implement it. I remember Samba implements
an interruptible select() using a wait on multiple object (equivalent)
by waiting on the actual socket/pipe and another pipe. Any signal
handler writes a byte into the other pipe. The wait on multiple object
returns and since we know which fd had data, we know it was due to an
interrupt/signal. A similar implementation could be used to implement
a cancel able IO, select().

-dky

-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Eli Zaretskii]

> Date: Thu, 16 Aug 2007 19:39:11 +0530
> From: dhruva <dhruvakm@gmail.com>
> Cc: Glenn Morris <rgm@gnu.org>, rms@gnu.org, emacs-devel@gnu.org
> 
> E:\cache\build\emacs\src
> [dky]hg cdiff w32proc.c
> *** emacs.58583440b48b\src\w32proc.c    2007-08-16 19:11:29.218750000 +0530
> --- E:\cache\build\emacs\src\w32proc.c  2007-08-16 19:02:44.828125000 +0530

Thanks, but please always use forward slashes when posting diffs here.
Diffs with backslashes cannot be applied on GNU/Linux and other Posix
platforms.

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Richard Stallman]

     I have put it a fix that seems to solve this problem (observed on my
    box).

Thanks for working on this.  In a few days, when you find out whether
the fix really works right, please tell me by responding to this message.

Please install your fix in EMACS_22_BASE.

    I find the issue in using SetEvent() and not waiting long enough
    before calling TerminateThread().

In the GNU Project we reject the Unix practice of writing "()"
to indicate that something is a function name.

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Richard Stallman]

    I can reproduce this on Emacs from CVS HEAD on XP using MinGW build
    with latest GCC 4. series.

Can you debug why it fails?

    However, I cannot reproduce with the VS.NET (MS Visual Studio 2003) build.

In that case, don't worry about VS.NET.  To debug a problem, all you
need is one case where it DOES reliably fail.

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

Hi,

On 8/17/07, Richard Stallman <rms@gnu.org> wrote:
>      I have put it a fix that seems to solve this problem (observed on my
>     box).
>
> Thanks for working on this.  In a few days, when you find out whether
> the fix really works right, please tell me by responding to this message.

My usage of Emacs with lot of subprocesses is highly limited (but I am
creating such scenarios and testing), I therefore request a wider
testing. For anyone willing to try a new patch, I am posting it here.
The rationale for reverting the changes done in my earlier patch is as
follows. Since I am a novice in contributing patches, I request fellow
developers guide me (as you have all been doing):

1. In the delete_child, I had increased the wait's timeout from 1000ms
to INFINITE. Since this gets called in the main thread, it is not a
good idea to wait infinitely on a kernel object.
2. If something goes wrong in the thread on which the main thread
waits, it will result in a deadlock and the process hangs

The new changes:
1. The new process (perl.exe in this example) is created in a new
thread and the thread entry function is reader_thread
2. The blocking calls in this new thread which handles the IO from the
sub process is through a call to _sys_read_ahead. This behaves like a
select by reading a single byte.
3. The _read call is synchronous and non-interruptible.
4. Hence, to make that function return when we want to shut down a
process, I close the pipe on which the read is happening from the main
thread. The call to _read returns with failure.
5. I am using PulseEvent instead of SetEvent as the former ensures
releasing of threads waiting on that event. This could bring the much
required delay before the main thread waits for the other thread's
termination.
6. An additional Sleep is called to relinquish the CPU quanta so that
a different thread could get scheduled. There is a higher probability
of the thread waiting on the event (which the main thread has
signaled) getting scheduled before the main thread gets the next time
slice (based on what little I know of the windows scheduler).
7. If all fails, we do not wait infinitely (as was in my previous patch).

diff -r 8ea8d2856c72 src/w32proc.c
--- a/src/w32proc.c     Thu Aug 16 20:46:34 2007 +0000
+++ b/src/w32proc.c     Fri Aug 17 10:43:58 2007 +0530
@@ -215,7 +215,13 @@ delete_child (child_process *cp)
         {
          /* let the thread exit cleanly if possible */
          cp->status = STATUS_READ_ERROR;
-         SetEvent (cp->char_consumed);
+         /* Close the pipe so that the blocking _sys_read_ahead fails
+            and returns */
+         close (cp->fd);
+         /* Ensure the waiting threads are released before proceeding */
+         PulseEvent (cp->char_consumed);
+         /* Give the other thread an opportunity to exit gracefully */
+         Sleep (0);
          if (WaitForSingleObject (cp->thrd, 1000) != WAIT_OBJECT_0)
            {
              DebPrint (("delete_child.WaitForSingleObject (thread) failed "


-dky

-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Jason Rumney]

dhruva wrote:
> Hi,
>  I have put it a fix that seems to solve this problem
> !         if (WaitForSingleObject (cp->thrd, 1000) != WAIT_OBJECT_0)
>   

Why would WaitSingleObject act differently depending on which compiler
was use to compile Emacs? It is a system call. I think the problem lies
elsewhere, and this is just a workaround (which could itself lead to
indefinite hanging of Emacs).

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

Hi,

On 8/17/07, Jason Rumney <jasonr@gnu.org> wrote:
> dhruva wrote:
> > Hi,
> >  I have put it a fix that seems to solve this problem
> > !         if (WaitForSingleObject (cp->thrd, 1000) != WAIT_OBJECT_0)
> >
>
> Why would WaitSingleObject act differently depending on which compiler
> was use to compile Emacs? It is a system call. I think the problem lies
> elsewhere, and this is just a workaround (which could itself lead to
> indefinite hanging of Emacs).

I have not modified the the call to WaitForSingleObject in the latest
patch. I doubt the implementation of read which could be different for
different compilers (C-runtime library).
It is the call to read in _sys_read_ahead which continues to block
even after the sub process is killed. If we change that read to use
ReadFile using OVERLAPPED structure, we could handle canceling of IO
when we want to terminate the thread more elegantly. But, that would
be more changes as we need to use the HANDLE and not fd.

-dky

-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Jason Rumney]

dhruva wrote:
> I have not modified the the call to WaitForSingleObject in the latest
> patch. I doubt the implementation of read which could be different for
> different compilers (C-runtime library).
>   

It may be implemented as a macro in one or both compilers, that would
explain it. The C runtime library is the same system library in both
cases (CRTDLL.DLL)

> It is the call to read in _sys_read_ahead which continues to block
> even after the sub process is killed. If we change that read to use
> ReadFile using OVERLAPPED structure, we could handle canceling of IO
> when we want to terminate the thread more elegantly. But, that would
> be more changes as we need to use the HANDLE and not fd.
>   

This would be a good change, not just here, but in sys_select in
w32proc.c, which is overcomplicated due to a past design decision to use
winsock 1.1 rather than winsock 2. We have changed that in 22.1 since
the versions of Windows NT that lack winsock 2 support have not been
supported by Emacs for some years now.

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]

Hi,

On 8/18/07, Jason Rumney <jasonr@gnu.org> wrote:
> dhruva wrote:
> > It is the call to read in _sys_read_ahead which continues to block
> > even after the sub process is killed. If we change that read to use
> > ReadFile using OVERLAPPED structure, we could handle canceling of IO
> > when we want to terminate the thread more elegantly. But, that would
> > be more changes as we need to use the HANDLE and not fd.
> >
>
> This would be a good change, not just here, but in sys_select in
> w32proc.c, which is overcomplicated due to a past design decision to

I have made a new patch using ASYNC Read in _sys_read_ahead. We now
have an IO that can be canceled. With this, the call to
TerminateThread in the delete_child in w32proc.c should ideally never
happen. Once this gets well tested, I will try to implement similar
ASYNC mechanisms in sys_select.
 In the patch, I have made a call to memset in the _sys_read_ahead.
Since we read char at a time, this could be considered as an overhead.
I did not want to
Since I have XP, I request others to test the patch on different
supported windows flavors of emacs. I have tested with the reproducer
(calling perl), invoking a command shell and typing various dos
commands. I have not noticed any failures so far.

-dky

-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

[-- Attachment #2: async.patch --]
[-- Type: application/octet-stream, Size: 3228 bytes --]

diff -r 00b25744642d -r 6a7768a60186 src/w32.c
--- a/src/w32.c	Sat Aug 18 08:38:05 2007 +0000
+++ b/src/w32.c	Sun Aug 19 13:05:35 2007 +0530
@@ -3662,7 +3662,7 @@ _sys_read_ahead (int fd)
 _sys_read_ahead (int fd)
 {
   child_process * cp;
-  int rc;
+  int rc = -1;
 
   if (fd < 0 || fd >= MAXDESC)
     return STATUS_READ_ERROR;
@@ -3683,7 +3683,30 @@ _sys_read_ahead (int fd)
 
   if (fd_info[fd].flags & FILE_PIPE)
     {
-      rc = _read (fd, &cp->chr, sizeof (char));
+      OVERLAPPED ov;
+      ov.Offset=0;
+      ov.OffsetHigh=0;
+      ov.hEvent=cp->char_avail;
+
+      /* Use asynchronous IO to graciously handle stop events */
+      HANDLE h_file=_get_osfhandle (fd);
+      if(h_file && ReadFile (h_file, &cp->chr, sizeof(char), NULL, &ov))
+	{
+	  /* Wait on multiple events - ASYNC read & stop event */
+	  DWORD w_status = 0;
+	  HANDLE aHandles[2] = {cp->char_avail,cp->stop_event};
+	  w_status = WaitForMultipleObjects (2, aHandles, FALSE, INFINITE);
+	  if(WAIT_OBJECT_0 == w_status)
+	    {
+	      DWORD bytesRead=0;
+	      if(GetOverlappedResult (h_file, &ov, &bytesRead, FALSE))
+		rc=(int)bytesRead;
+	    }
+	  else if(WAIT_OBJECT_0+1 == w_status)
+	    {
+	      CancelIo (h_file);
+	    }
+	}
 
       /* Give subprocess time to buffer some more output for us before
 	 reporting that input is available; we need this because Windows 95
diff -r 00b25744642d -r 6a7768a60186 src/w32.h
--- a/src/w32.h	Sat Aug 18 08:38:05 2007 +0000
+++ b/src/w32.h	Sun Aug 19 13:05:35 2007 +0530
@@ -69,6 +69,7 @@ typedef struct _child_process
   int                   pid;
   HANDLE                char_avail;
   HANDLE                char_consumed;
+  HANDLE                stop_event;
   HANDLE                thrd;
   HWND                  hwnd;
   PROCESS_INFORMATION   procinfo;
diff -r 00b25744642d -r 6a7768a60186 src/w32proc.c
--- a/src/w32proc.c	Sat Aug 18 08:38:05 2007 +0000
+++ b/src/w32proc.c	Sun Aug 19 13:05:35 2007 +0530
@@ -183,10 +183,14 @@ new_child (void)
     {
       cp->char_consumed = CreateEvent (NULL, FALSE, FALSE, NULL);
       if (cp->char_consumed)
-        {
-	  cp->thrd = CreateThread (NULL, 1024, reader_thread, cp, 0, &id);
-	  if (cp->thrd)
-	    return cp;
+	{
+	  cp->stop_event = CreateEvent (NULL, TRUE, FALSE, NULL);
+	  if (cp->stop_event)
+	    {
+	      cp->thrd = CreateThread (NULL, 1024, reader_thread, cp, 0, &id);
+	      if (cp->thrd)
+		return cp;
+	    }
 	}
     }
   delete_child (cp);
@@ -215,6 +219,8 @@ delete_child (child_process *cp)
         {
 	  /* let the thread exit cleanly if possible */
 	  cp->status = STATUS_READ_ERROR;
+	  /* Inform the reader_thread about intentions to shutdown */
+	  PulseEvent (cp->stop_event);
 	  SetEvent (cp->char_consumed);
 	  if (WaitForSingleObject (cp->thrd, 1000) != WAIT_OBJECT_0)
 	    {
@@ -235,6 +241,11 @@ delete_child (child_process *cp)
     {
       CloseHandle (cp->char_consumed);
       cp->char_consumed = NULL;
+    }
+  if (cp->stop_event)
+    {
+      CloseHandle (cp->stop_event);
+      cp->stop_event = NULL;
     }
 
   /* update child_proc_count (highest numbered slot in use plus one) */

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

Hi,
 My apologies for sending a rather big patch. I did not notice till I
decided to import it in a different repository. I had taken the diff
by not ignoring the whitespaces. Apologies once again for the
inconvenience caused.

-dky

-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[dhruva]

Hi,
 I have found the below patch to be the simplest and safe fix. I feel
it was just an issue if timing, thread trying to terminate on it's own
and a call the TerminateThread.
 We really do not have to terminate the thread. The sys_select closes
the pipe to any non-existent process. The _sys_read_ahead will fail
which will ensure the thread will terminate normally (graciously). It
may take an extra loop but seems the cleanest way to handle. I have
applied this patch and have used it for over a week, I keep creating
sub processes for invoking shell and git/hg and have seen no problems.
I have also tried with the OP example of launching perl and killing it
in a loop too.


diff --git a/src/w32proc.c b/src/w32proc.c
index adf5152..d4642a7 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -216,12 +216,14 @@ delete_child (child_process *cp)
          /* let the thread exit cleanly if possible */
          cp->status = STATUS_READ_ERROR;
          SetEvent (cp->char_consumed);
+#if 0
          if (WaitForSingleObject (cp->thrd, 1000) != WAIT_OBJECT_0)
            {
              DebPrint (("delete_child.WaitForSingleObject (thread) failed "
                         "with %lu for fd %ld\n", GetLastError (), cp->fd));
              TerminateThread (cp->thrd, 0);
            }
+#endif
        }
       CloseHandle (cp->thrd);
       cp->thrd = NULL;

On 8/17/07, Richard Stallman <rms@gnu.org> wrote:
>      I have put it a fix that seems to solve this problem (observed on my
>     box).
>
> Thanks for working on this.  In a few days, when you find out whether
> the fix really works right, please tell me by responding to this message.
>
> Please install your fix in EMACS_22_BASE.

IMHO, we could have the above patch in the mainstream. If someone else
wants to test it, please do so before committing in the changes.

-dky

-- 
Dhruva Krishnamurthy
Contents reflect my personal views only!

Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process]

[Jason Rumney]

dhruva wrote:
> Hi,
>  I have found the below patch to be the simplest and safe fix.
Thanks, I have installed this in EMACS_22_BASE.