bug#56025: 29.0.50; em-extpipe-test-2 times out on EMBA and Cygwin

[Ken Brown <kbrown@cornell.edu>]

On 6/27/2022 3:18 PM, Jim Porter wrote:
> On 6/27/2022 6:25 AM, Ken Brown wrote:
>> Thanks to all of you for working on this while I was gone. Unfortunately, the 
>> problem is still present on Cygwin.  In my haste to get away, I neglected to 
>> mention that there is apparently a timing issue in Eshell on Cygwin, so that 
>> even three EOFs do not always suffice to kill the process.
>>
>> My test case is to run
>>
>>    echo bar | sh -c rev
>>
>> in Eshell.  For reasons I don't understand, EOF almost always has to be sent 
>> more than 3 times times before the "sh" process dies.  The maximum I've 
>> observed is 93.  Inserting "(sit-for 0.01)" after each EOF eliminates the need 
>> for extra EOFs; this is why I referred to the problem as a timing issue.
>>
>> I propose the following workaround:
>>
>> --- a/lisp/eshell/esh-io.el
>> +++ b/lisp/eshell/esh-io.el
>> @@ -284,10 +284,16 @@ eshell-close-target
>>       ;; end-of-file to the reading process.  However, some platforms
>>       ;; (e.g. Solaris) actually require sending a *third* EOF.  Since
>>       ;; sending extra EOFs while the process is running shouldn't break
>> -    ;; anything, we'll just send the maximum we'd ever need.  See
>> -    ;; bug#56025 for further details.
>> -    (let ((i 0))
>> -      (while (and (<= (cl-incf i) 3)
>> +    ;; anything, we'll send up to three on all platforms.
>> +
>> +    ;; There's an extra wrinkle on Cygwin where, apparently due to an
>> +    ;; unknown timing issue, it sometimes takes more than three EOFs
>> +    ;; to kill the process.  (This only happens in Eshell, not in an
>> +    ;; ordinary Cygwin shell.)  We work around this problem by sending
>> +    ;; up to 1000 EOFs on Cygwin.  See bug#56025 for further details.
>> +    (let ((i 0)
>> +          (n (if (eq system-type 'cygwin) 1000 3)))
>> +      (while (and (<= (cl-incf i) n)
>>                     (eq (process-status target) 'run))
>>           (process-send-eof target))))
> 
> I'd be very hesitant to do this, since as you mention above, this seems like a 
> timing issue, and it's entirely possible that there are other, more widespread 
> issues on Cygwin here. We'd also want to check the system that the process is 
> actually running on; otherwise, remoting into a Cygwin system (via Tramp) would 
> still exhibit the problem. I'll see if I can get a Cygwin environment up to test 
> things out in the next week-ish.

OK, thanks.  Let me know if you need any help with that.

> If there's no other way that we can come up with here, I'd lean towards a 
> defcustom so that users can tweak this if needed.

Sounds good.

Ken