The case against until_no_eintr

Martin (gzlist) gzlist at googlemail.com
Sat Feb 20 00:19:08 GMT 2010


I realise I may not have conveyed, beyond the "it's currently really
broken" why I think the until_no_eintr approach is a bad idea.

Andrew has mentioned in the merge request for my first pass at a
different method[1] that the sendall-with-bug-workarounds should be a
public function, and there's a use of sendall in bzrlib.transport.ftp
that could be using it as well. Well, let's take another example from
the same module:

    f.storbinary('STOR '+tmp_abspath, fp)

So, pretend we've got a bug report on from a user that ends in a EINTR
socket.error with a traceback going through there. But it's okay!
We've got a handy function in osutils to make it all better:

    osutils.until_no_eintr(f.storbinary('STOR '+tmp_abspath, fp))

Let's just check what ftplib is doing exactly...

    def storbinary(self, cmd, fp, blocksize=8192):
        '''Store a file in binary mode.'''
        self.voidcmd('TYPE I')
        conn = self.transfercmd(cmd)
        while 1:
            buf = fp.read(blocksize)
            if not buf: break
            conn.sendall(buf)
        conn.close()
        return self.voidresp()

Whoops.

Using until_no_eintr takes a narrow problem with sockets and pipes and
spreads it all over the standard library and all over bzrlib.

Peronally, I'd be happy with an 80% approach that addresses only those
places where interruption is actually likely as opposed to
theoretically possible. What really needs to be avoided are fixes that
turn a rare exception into silent loss or duplication of data on the
wire. Bazaar can (and already has to for other reasons) recover from a
connection being dropped unexpectedly, and there's only so much
robustness than can be expected from Python (or C) code.

So, though it's now been shown we have to worry about subprocesses as
well as sockets, I think scattering wrappers all over the tree isn't a
great plan.

Martin


[1]: https://code.launchpad.net/~gz/bzr/no_until_no_eintr/+merge/19615



More information about the bazaar mailing list