Incorrect handling of EINTR in Bazaar

[Martin (gzlist)]

On 16/02/2010, Martin Pool <mbp at canonical.com> wrote:
>
> We still support python back to 2.4; considering the deployed
> environment of interpreters we prefer to work around bugs in Python if
> at all feasible.

The existing code does not work on either Python 2.4 or Python 2.5 so
a backout can't do any harm there. Whether it's worth addressing this
(mostly harmless) corner case on old versions is your call, getting
the solid fix in a 2.6 dot release is by far the cleanest option.

>> There are no tests. Instigating workarounds without anything to verify
>> they are in fact correct means the other issues described are not
>> detected.
>
> That's true (on both counts.)  However, this is a bit of an
> interesting case to test: one could in principle write objects that
> insert EINTR calls, but if the actual problem is a misunderstanding of
> interaction with the OS, then the tests are likely to reproduce the
> same misunderstanding.

Also true, and the sendall issue in particular is difficult to expose
in testing. However, it would have caught the socket.error discrepancy
across versions.

> Yes, but isn't it harmless to check for them?
>
> Similarly, I raised the issue in review that some python calls already
> handle eintr or partial reads, but it seems harmless to check for it.

There is some harm, first it still causes the bugs as with module
cleanup, second it misleads people reading the code. Keeping a few
catches that won't do anything on some platforms is reasonable though.

> So perhaps we should have a decorator object that does handle eintr
> and install that only when needed?

The problem is the check is needed in several cases where there is
nothing you can do in client code but die, as it's not possible to
resume. There are options when it comes to a complete fix, I can
prepare a couple of branches for review and y'all can pick one.

> Can you be more specific about what you want to see backed out, and
> what approach you think we should have instead?

Everything but the NEWS from the two merges I linked; reporting and
fixing upstream bugs upstream. Even if it means then adapting a fix
into a monkey-patch for previous versions, it gets the change looked
at by the right people.

Martin