Incorrect handling of EINTR in Bazaar

[Martin Pool]

On 20 February 2010 11:13, Martin (gzlist) <gzlist at googlemail.com> wrote:
> On 19/02/2010, Martin Pool <mbp at canonical.com> wrote:
>>>
>>> Anyway, you're right, we can't usefully catch EINTR from Python in this
>>> case.  Well, actually, we could remember the fileno, and if EINTR occurs
>>> then until_no_eintr(os.close, fileno), but that's probably not worth the
>>> effort just to avoid leaking an fd in this corner case.
>>
>> No, actually I think it is important that you handle this, because
>> close has larger effects than just closing the filehandle.  If it is
>> never successfully called then eg the network socket may not close,
>> and this is just the kind of case where you are likely to see eintr.
>
> Okay, this is the sort of hairy territory I think should be avoided
> without being looked at upstream first.
> For pipes, the glibc
> documentation has an explicit warning that fclose should be used with
> streams to make sure the internal buffers are flushed etc. And for
> sockets, there are several platforms where os.close uses a different
> function from the one socket.close uses. It would be all too easy to
> write a 'workaround' that was actually far less robust than the base
> function it was replacing.

My main point to Andrew is that you can't just assume that the worst
that will happen by forgetting to close is that an fd will leak, so we
seem to be agreeing :-)

Any particular function will be in one of these categories:

1- never raises eintr
2- raises eintr and must be restarted if it occurs
3- behaves incorrectly if an underlying function raises eintr; eg it
propagates the error but cannot safely be restarted

Generally speaking the partial-read/write behaviour aligns with eintr.

At the unix and libc level, there are many functions in category 1 and
2 and it would be highly surprising to find one in the third.
Assuming the eintr handler is correctly written it is safe to put them
on any of these, and the worst that can happen is it will be redundant
for class 1 functions.

In python, sadly, it looks like there are many functions in the third
category, such as the ftp one you mentioned.  In this case it is
better to just crash than to restart incorrectly.

We do actually have siginterrupt exposed in Python - maybe that can be
used to cause system calls to automatically restart? (See
<http://manpages.ubuntu.com/manpages/lucid/en/man3/siginterrupt.3.html>
<http://bugs.python.org/issue1089358>).  We would still have to handle
partial io.

-- 
Martin <http://launchpad.net/~mbp/>