* [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] @ 2007-08-15 18:21 Richard Stallman 2007-08-16 3:24 ` Glenn Morris 0 siblings, 1 reply; 18+ messages in thread From: Richard Stallman @ 2007-08-15 18:21 UTC (permalink / raw) To: emacs-devel 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 ------- ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-15 18:21 [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] Richard Stallman @ 2007-08-16 3:24 ` Glenn Morris 2007-08-16 4:46 ` dhruva 2007-08-16 8:02 ` Jason Rumney 0 siblings, 2 replies; 18+ messages in thread From: Glenn Morris @ 2007-08-16 3:24 UTC (permalink / raw) To: rms; +Cc: emacs-devel 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 :-( ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 3:24 ` Glenn Morris @ 2007-08-16 4:46 ` dhruva 2007-08-17 0:32 ` Richard Stallman 2007-08-16 8:02 ` Jason Rumney 1 sibling, 1 reply; 18+ messages in thread From: dhruva @ 2007-08-16 4:46 UTC (permalink / raw) To: Glenn Morris; +Cc: rms, emacs-devel 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! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 4:46 ` dhruva @ 2007-08-17 0:32 ` Richard Stallman 0 siblings, 0 replies; 18+ messages in thread From: Richard Stallman @ 2007-08-17 0:32 UTC (permalink / raw) To: dhruva; +Cc: rgm, emacs-devel 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 3:24 ` Glenn Morris 2007-08-16 4:46 ` dhruva @ 2007-08-16 8:02 ` Jason Rumney 2007-08-16 12:42 ` dhruva 1 sibling, 1 reply; 18+ messages in thread From: Jason Rumney @ 2007-08-16 8:02 UTC (permalink / raw) To: Glenn Morris; +Cc: rms, emacs-devel 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 8:02 ` Jason Rumney @ 2007-08-16 12:42 ` dhruva 2007-08-16 14:09 ` dhruva 0 siblings, 1 reply; 18+ messages in thread From: dhruva @ 2007-08-16 12:42 UTC (permalink / raw) To: Jason Rumney; +Cc: Glenn Morris, rms, emacs-devel 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! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 12:42 ` dhruva @ 2007-08-16 14:09 ` dhruva 2007-08-16 17:04 ` dhruva ` (3 more replies) 0 siblings, 4 replies; 18+ messages in thread From: dhruva @ 2007-08-16 14:09 UTC (permalink / raw) To: Jason Rumney; +Cc: Glenn Morris, rms, emacs-devel 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! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 14:09 ` dhruva @ 2007-08-16 17:04 ` dhruva 2007-08-16 19:41 ` Eli Zaretskii ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: dhruva @ 2007-08-16 17:04 UTC (permalink / raw) To: Emacs Devel 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! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 14:09 ` dhruva 2007-08-16 17:04 ` dhruva @ 2007-08-16 19:41 ` Eli Zaretskii 2007-08-17 0:32 ` Richard Stallman 2007-08-17 13:43 ` Jason Rumney 3 siblings, 0 replies; 18+ messages in thread From: Eli Zaretskii @ 2007-08-16 19:41 UTC (permalink / raw) To: dhruva; +Cc: emacs-devel > 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 14:09 ` dhruva 2007-08-16 17:04 ` dhruva 2007-08-16 19:41 ` Eli Zaretskii @ 2007-08-17 0:32 ` Richard Stallman 2007-08-17 5:27 ` dhruva 2007-08-27 10:07 ` dhruva 2007-08-17 13:43 ` Jason Rumney 3 siblings, 2 replies; 18+ messages in thread From: Richard Stallman @ 2007-08-17 0:32 UTC (permalink / raw) To: dhruva; +Cc: rgm, emacs-devel, jasonr 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-17 0:32 ` Richard Stallman @ 2007-08-17 5:27 ` dhruva 2007-08-27 10:07 ` dhruva 1 sibling, 0 replies; 18+ messages in thread From: dhruva @ 2007-08-17 5:27 UTC (permalink / raw) To: rms; +Cc: rgm, emacs-devel, jasonr 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! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-17 0:32 ` Richard Stallman 2007-08-17 5:27 ` dhruva @ 2007-08-27 10:07 ` dhruva 2007-09-02 17:02 ` Jason Rumney 1 sibling, 1 reply; 18+ messages in thread From: dhruva @ 2007-08-27 10:07 UTC (permalink / raw) To: rms; +Cc: rgm, emacs-devel, jasonr 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! ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-27 10:07 ` dhruva @ 2007-09-02 17:02 ` Jason Rumney 0 siblings, 0 replies; 18+ messages in thread From: Jason Rumney @ 2007-09-02 17:02 UTC (permalink / raw) To: dhruva; +Cc: rgm, rms, emacs-devel 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-16 14:09 ` dhruva ` (2 preceding siblings ...) 2007-08-17 0:32 ` Richard Stallman @ 2007-08-17 13:43 ` Jason Rumney 2007-08-17 17:31 ` dhruva 3 siblings, 1 reply; 18+ messages in thread From: Jason Rumney @ 2007-08-17 13:43 UTC (permalink / raw) To: dhruva; +Cc: Glenn Morris, rms, emacs-devel 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). ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-17 13:43 ` Jason Rumney @ 2007-08-17 17:31 ` dhruva 2007-08-17 20:40 ` Jason Rumney 0 siblings, 1 reply; 18+ messages in thread From: dhruva @ 2007-08-17 17:31 UTC (permalink / raw) To: Jason Rumney; +Cc: Glenn Morris, rms, emacs-devel 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! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-17 17:31 ` dhruva @ 2007-08-17 20:40 ` Jason Rumney 2007-08-19 7:42 ` dhruva 0 siblings, 1 reply; 18+ messages in thread From: Jason Rumney @ 2007-08-17 20:40 UTC (permalink / raw) To: dhruva; +Cc: Glenn Morris, rms, emacs-devel 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-17 20:40 ` Jason Rumney @ 2007-08-19 7:42 ` dhruva 2007-08-22 6:29 ` dhruva 0 siblings, 1 reply; 18+ messages in thread From: dhruva @ 2007-08-19 7:42 UTC (permalink / raw) To: Jason Rumney; +Cc: Glenn Morris, rms, emacs-devel [-- 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] 2007-08-19 7:42 ` dhruva @ 2007-08-22 6:29 ` dhruva 0 siblings, 0 replies; 18+ messages in thread From: dhruva @ 2007-08-22 6:29 UTC (permalink / raw) To: Emacs Devel 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! ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2007-09-02 17:02 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-08-15 18:21 [andrew.maguire@ge.com: Emacs 22.1 hung after delete-process] Richard Stallman 2007-08-16 3:24 ` Glenn Morris 2007-08-16 4:46 ` dhruva 2007-08-17 0:32 ` Richard Stallman 2007-08-16 8:02 ` Jason Rumney 2007-08-16 12:42 ` dhruva 2007-08-16 14:09 ` dhruva 2007-08-16 17:04 ` dhruva 2007-08-16 19:41 ` Eli Zaretskii 2007-08-17 0:32 ` Richard Stallman 2007-08-17 5:27 ` dhruva 2007-08-27 10:07 ` dhruva 2007-09-02 17:02 ` Jason Rumney 2007-08-17 13:43 ` Jason Rumney 2007-08-17 17:31 ` dhruva 2007-08-17 20:40 ` Jason Rumney 2007-08-19 7:42 ` dhruva 2007-08-22 6:29 ` dhruva
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.