Bug in poll/select readable state on write end of pipe

Bug in poll/select readable state on write end of pipe

[Erik Bray]

Hi,

I encountered this bug through a hang in Twisted, but narrowed it down
to a simple example (in Python here, but the same would work in C).

Basically, in Twisted's system process runner, there's a "hack" [1]
which basically assumes readability (as in a poll() loop) of the
writable end of a pipe indicates that the pipe is closed and/or
there's an error.  Although the "hack" is disabled by default, in
practice it is always forced to be enabled [2].  One could maybe
consider this a bug in Twisted since I can't find anything in POSIX
which states this behavior (though I could be missing it).

Nevertheless, on Linux this *is* the behavior:

>>> import os
>>> import select
>>> r, w = os.pipe()
>>> poller = select.poll()
>>> os.close(r)
>>> poller.register(w, select.POLLIN)
>>> print(poller.poll(1000))
[(4, 8)]

where 8 indicates that POLLERR is set on fd 4, indicating in this case
that if we tried to write to the pipe we would get a broken pipe
error.

However, on Cygwin the same code returns an empty list.  I don't know
if this *should* be fixed, but it would be nice.  It's slightly tricky
though.  In Cygwin's poll there's a line [3] that does something
similar for sockets--if the socket is not connected it sets POLLERR in
the results.  One could do something similar for pipes, but there
isn't an existing internal API to do this conveniently.  What one
might want is something that calls NtQueryInformationFile like in
pipe_data_available [4], and checks the NamedPipeState flag.  But that
something doesn't exist yet.

Any ideas?

Erik


[1] https://github.com/twisted/twisted/blob/bb371b3b602b1547c6e603d3b234bb18ded6d67c/src/twisted/internet/process.py#L139

[2] https://github.com/twisted/twisted/blob/bb371b3b602b1547c6e603d3b234bb18ded6d67c/src/twisted/internet/process.py#L764

[3] https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/poll.cc;h=ea45b70adacaca96edbc46d7b8ffe1cd8a94270e;hb=HEAD#l122

[4] https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/select.cc;h=f5a993850408477a65d9ecda444785d0db1f35a8;hb=HEAD#l567

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Bug in poll/select readable state on write end of pipe

[Corinna Vinschen]

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

On Nov  7 17:39, Erik Bray wrote:
> Hi,
> 
> I encountered this bug through a hang in Twisted, but narrowed it down
> to a simple example (in Python here, but the same would work in C).
> 
> Basically, in Twisted's system process runner, there's a "hack" [1]
> which basically assumes readability (as in a poll() loop) of the
> writable end of a pipe indicates that the pipe is closed and/or
> there's an error.  Although the "hack" is disabled by default, in
> practice it is always forced to be enabled [2].  One could maybe
> consider this a bug in Twisted since I can't find anything in POSIX
> which states this behavior (though I could be missing it).
> 
> Nevertheless, on Linux this *is* the behavior:
> 
> >>> import os
> >>> import select
> >>> r, w = os.pipe()
> >>> poller = select.poll()
> >>> os.close(r)
> >>> poller.register(w, select.POLLIN)
> >>> print(poller.poll(1000))
> [(4, 8)]
> 
> where 8 indicates that POLLERR is set on fd 4, indicating in this case
> that if we tried to write to the pipe we would get a broken pipe
> error.
> 
> However, on Cygwin the same code returns an empty list.  I don't know
> if this *should* be fixed, but it would be nice.  It's slightly tricky
> though.  In Cygwin's poll there's a line [3] that does something
> similar for sockets--if the socket is not connected it sets POLLERR in
> the results.  One could do something similar for pipes, but there
> isn't an existing internal API to do this conveniently.  What one
> might want is something that calls NtQueryInformationFile like in
> pipe_data_available [4], and checks the NamedPipeState flag.  But that
> something doesn't exist yet.
> 
> Any ideas?

What about utilizing pipe_data_available for just such an emergency(*).

There's a bit of cleanup in pipe_data_available necessary, but nothing
big:

- An int function returning "true", "false", and -1?  Ouch.

- NtQueryInformationFile is just checked for a non-0 return value
  but it should actually be checked against NT_SUCCESS.

But other than that, it should do the trick, shouldn't it?


Corinna


(*) Sorry for the "All Fowled up" pun.

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]