Can we apply this patch for bug 84 and 425?

Can we apply this patch for bug 84 and 425?

[joakim]

These bugs are both about hangs when emacs deals with external processes:
   http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=84
   http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=425

Markus Triska has a patch which reduces my experiences of the hang from
about once a day to not at all so far.

How can we proceed with this patch? 

diff --git a/src/process.c b/src/process.c
index e8d0090..0cf04bc 100644
--- a/src/process.c
+++ b/src/process.c
@@ -4791,6 +4791,10 @@ wait_reading_process_output (time_limit, microsecs, read_kbd, do_display,
 	      else if (nread == -1 && EWOULDBLOCK == errno)
                 break;
 #endif
+#ifdef EBADF
+	      else if (nread == -1 && EBADF == errno)
+		break;
+#endif
 	    }
 	  if (total_nread > 0 && do_display)
 	    redisplay_preserve_echo_area (10);


   
-- 
Joakim Verona

Re: Can we apply this patch for bug 84 and 425?

[Chong Yidong]

joakim@verona.se writes:

> These bugs are both about hangs when emacs deals with external processes:
>    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=84
>    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=425
>
> Markus Triska has a patch which reduces my experiences of the hang
> from about once a day to not at all so far.
>
> How can we proceed with this patch? 

According to Markus in the discussion in #425, these two bugs were due
to #562, which was fixed by undefining HAVE_RES_INIT on darwin.  Is this
patch still necessary?

Re: Can we apply this patch for bug 84 and 425?

[Stefan Monnier]

> These bugs are both about hangs when emacs deals with external processes:
>    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=84
>    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=425

> Markus Triska has a patch which reduces my experiences of the hang from
> about once a day to not at all so far.

> How can we proceed with this patch? 

> diff --git a/src/process.c b/src/process.c
> index e8d0090..0cf04bc 100644
> --- a/src/process.c
> +++ b/src/process.c
> @@ -4791,6 +4791,10 @@ wait_reading_process_output (time_limit, microsecs, read_kbd, do_display,
>  	      else if (nread == -1 && EWOULDBLOCK == errno)
>                  break;
>  #endif
> +#ifdef EBADF
> +	      else if (nread == -1 && EBADF == errno)
> +		break;
> +#endif
>  	    }
>  	  if (total_nread > 0 && do_display)
>  	    redisplay_preserve_echo_area (10);

This patch is not acceptable as-is: EBADF should normally be a symptom
of another bug in Emacs (we should never pass a bad descriptor), so
rather than ignore it, we should figure out where it comes from and fix
it there.


        Stefan

Re: Can we apply this patch for bug 84 and 425?

[Chong Yidong]

joakim@verona.se writes:

> Chong Yidong <cyd@stupidchicken.com> writes:
>
>> joakim@verona.se writes:
>>
>>> These bugs are both about hangs when emacs deals with external processes:
>>>    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=84
>>>    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=425
>>> Markus Triska has a patch which reduces my experiences of the hang
>>> from about once a day to not at all so far.
>>>
>>> How can we proceed with this patch? 
>>
>> According to Markus in the discussion in #425, these two bugs were due
>> to #562, which was fixed by undefining HAVE_RES_INIT on darwin.  Is this
>> patch still necessary?
>
> Well I still experience the problem without Marcus patch, on Fedora core
> 9, i686 GNU/Linux.

I see.

The patch looks fairly safe; I think it can be checked in.

Re: Can we apply this patch for bug 84 and 425?

[Chong Yidong]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> This patch is not acceptable as-is: EBADF should normally be a symptom
> of another bug in Emacs (we should never pass a bad descriptor), so
> rather than ignore it, we should figure out where it comes from and
> fix it there.

Hmm, the bug seen by Joakim happens on GNU/Linux, so it can't be related
to the RES_INIT issues leading to #84 and #425.  Joakim, could you try
to debug where the bad descriptor comes from?

Re: Can we apply this patch for bug 84 and 425?

[joakim]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> These bugs are both about hangs when emacs deals with external processes:
>>    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=84>    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=425
>
>> Markus Triska has a patch which reduces my experiences of the hang from
>> about once a day to not at all so far.
>
>> How can we proceed with this patch? 
>
>> diff --git a/src/process.c b/src/process.c
>> index e8d0090..0cf04bc 100644
>> --- a/src/process.c
>> +++ b/src/process.c
>> @@ -4791,6 +4791,10 @@ wait_reading_process_output (time_limit, microsecs, read_kbd, do_display,
>>  	      else if (nread == -1 && EWOULDBLOCK == errno)
>>                  break;
>>  #endif
>> +#ifdef EBADF
>> +	      else if (nread == -1 && EBADF == errno)
>> +		break;
>> +#endif
>>  	    }
>>  	  if (total_nread > 0 && do_display)
>>  	    redisplay_preserve_echo_area (10);
>
> This patch is not acceptable as-is: EBADF should normally be a symptom
> of another bug in Emacs (we should never pass a bad descriptor), so
> rather than ignore it, we should figure out where it comes from and fix
> it there.

Just to be fair to Marcus, he also said it needed further debugging to
determine the actual cause of the bug. OTOH I've tried to debug it and I
cant find a reproducible test-case. The bug just manifests sometimes.

>
>
>         Stefan
>
-- 
Joakim Verona

Re: Can we apply this patch for bug 84 and 425?

[Chong Yidong]

joakim@verona.se writes:

> Just to be fair to Marcus, he also said it needed further debugging to
> determine the actual cause of the bug. OTOH I've tried to debug it and I
> cant find a reproducible test-case. The bug just manifests sometimes.

Have you applying the patch in question to Emacs, running Emacs under
GDB, and setting the breakpoint at the `break;' line?

>>> +#ifdef EBADF
>>> +	      else if (nread == -1 && EBADF == errno)
>>> +		break;
>>> +#endif