Another pipe-related problem?

[Ken Brown]

On 11/9/2021 9:11 AM, Ken Brown via Cygwin wrote:
> On 11/9/2021 5:55 AM, Henry S. Thompson wrote:
>> As you may know, the XEmacs situation is complicated.  The old source
>> repo (bitbucket.org/xemacs) no longer exists.  There's a fork that's
>> still being maintained, but it's not widely publicised.  That's the
>> one I'm working with -- are you aware of this.
> 
> I was aware that the bitbucket repo didn't exist, because I tried to get the 
> sources there.  But I didn't know about the fork.  Please point me to it, or 
> just make a tarball available to me somehow.
> 
>> Here are the immediate contexts from the sources for the xemacs
>> sources in the above backtrace, might be enough to check your
>> hypothesis:
>>
>> sysdep.c:
>>
>>    retry_read_1 (int fildes, void *buf, size_t nbyte, int allow_quit)
>>    {
>>      ssize_t rtnval;
>>
>>      while ((rtnval = read (fildes, buf, nbyte)) == -1
>>             && (errno == EINTR))                         <<<<<<<<<<<<<<<<<<<<
>>        {
>>          if (allow_quit)
>>            QUIT;
>>        }
>>      return rtnval;
>>    }
> I'll have to reproduce the hang myself in order to test this (or maybe you could 
> test it), but I now have a new guess: If the read call above keeps failing with 
> EINTR, then we're in an infinite loop.  This could happen because of the 
> following code in fhandler_pipe::raw_read:
> 
>    DWORD waitret = cygwait (read_mtx, timeout);
>    switch (waitret)
>      {
>      case WAIT_OBJECT_0:
>        break;
>      case WAIT_TIMEOUT:
>        set_errno (EAGAIN);
>        len = (size_t) -1;
>        return;
>      default:
>        set_errno (EINTR);
>        len = (size_t) -1;
>        return;
>      }
> 
> Takashi, is EINTR really the appropriate errno in the default case?  Isn't 
> cygwait supposed to handle signals?

I was able to build XEmacs and reproduce the problem.  My guess was wrong, 
though my question to Takashi still stands.  I think the infinite loop is 
actually caused by a bug in fhandler_pipe::raw_read that only affects 
non-blocking pipes (which is what we have in XEmacs).

Consider the following code in fhandler_pipe::raw_read:

       status = NtReadFile (get_handle (), evt, NULL, NULL, &io, ptr,
			   len1, NULL, NULL);
       if (evt && status == STATUS_PENDING)      <<<<<<<<<<<<<<<<<<<<<<<<<<<<
	{
	  waitret = cygwait (evt, INFINITE, cw_cancel | cw_sig);
[...]
	}

In the non-blocking case, evt == NULL, but we still might have status == 
STATUS_PENDING.  We then should wait on get_handle() to let NtReadFile finish. 
By not waiting, we end up using a garbage value from io.Information, leading to 
an infinite loop in drain_signal_event_pipe.

I'll try to fix this.

Ken