[Twisted-Python] linux kernel 2.6.11-rc broke twisted process pipes

[James Y Knight]

On Feb 28, 2005, at 3:14 PM, Andrea Arcangeli wrote:
> I would like to understand exactly what broke twisted, so that we're
> sure the new 2.6.11 kernel will work reliably with twisted.
>
> This code especially will never trigger with 2.6.11 because we'll never
> set POLLIN|POLLOUT at the same time for writeable fd.
>
>     def doRead(self):
>         """The only way this pipe can become readable is at EOF, 
> because the
>         child has closed it.
>         """
>         fd = self.fd
>         r, w, x = select.select([fd], [fd], [], 0)
>         if r and w:
>             return CONNECTION_LOST
>
> Could you answer why anybody should call doRead on a writeable fd?
>
> Where is it written that at EOF a _write_ pipe channel becomes 
> readable?
>
> Is it ok that "r and w" will never trigger at the same time anymore,
> right? Was just that a superflous assumptions?
>
> Note that with 2.6.8 and all previous linux kernels, "r and w" could be
> == True in normal conditions too if the pipe write buffer was empty 
> (old
> kernel was setting pollin always for no good reason).
>
> What I suspect is that the above is a superflous check that was working
> by luck with older kernels (becasue we could set in and out at the same
> time even without a disconnect event), and that all you care about is 
> to
> get the wakeup from the write or disconnect events. And in turn the new
> linux 2.6.11-rc should not work by luck anymore.

I agree this code is somewhat suboptimal. However, I do not agree that 
it works only by luck.

In response to your main question of "why is it checking for 
readability at all", there is a good answer: to get the disconnect 
event without trying to write data. Select doesn't have a "err" fdset, 
so you have to select for either readability or writability. You can't 
select for writability when you have no data to write, or else you'll 
be continuously waking up. On all UNIX OSes that I know of, write pipes 
show up as readable in select when the other side has closed, for just 
this reason. I don't know if it's documented in any specs, but as far 
as I can tell, it's universally true.

Breaking this would be a Bad Thing, for I suspect more apps than just 
Twisted.

Of course, if it were that simple, doRead would just be implemented as 
"return CONNECTION_LOST". From my testing, that'd even work on BSDs. 
However, linux adds its own little wrinkle to the problem.

On linux, the observed behavior seems to be that only one write can be 
outstanding at a time -- if there is data in the buffer, writable will 
be false and readable will be true. Otherwise, the inverse. This is 
quite silly...but it's what happens. As far as I can tell, at no time 
are both true, except when the pipe is disconnected. On BSD, a write 
pipe is simply never readable until the reader is closed, which is a 
lot more sensible.

Also note, that bit of code is unnecessary if you're using pollreactor 
rather than selectreactor. That is, I think it should be fine with 
twisted if the pipe is just POLLHUP when it is disconnected rather than 
POLLHUP|POLLIN|POLLOUT.

James