[MERGE] Transport.readv() closes the file handle
John Arbash Meinel
john at arbash-meinel.com
Mon Oct 27 15:46:52 GMT 2008
-----BEGIN PGP SIGNED MESSAGE-----
> I *can* say that it is happening, and it is a bit hard to debug or test,
> because it only fails if you are doing something concurrently (which we
> don't often do in bzr) and if you are on a platform that doesn't like
> file handles left open.
> So if you prefer, I can make '.close()' a strictly required attribute,
> but I don't think I can assume all callers run the iterator to
> completion (because of the issue with zip()). I could track down all the
> different places to put readv() first, but rememember that sometimes
> generators are stacked, and that gets tricky to resolve.
I ended up going with this code:
> === modified file 'bzrlib/transport/__init__.py'
> --- bzrlib/transport/__init__.py 2008-10-06 06:34:36 +0000
> +++ bzrlib/transport/__init__.py 2008-10-27 15:28:44 +0000
> @@ -688,8 +688,21 @@
> # Now that we've read some data, see if we can yield anything back
> while cur_offset_and_size in data_map:
> this_data = data_map.pop(cur_offset_and_size)
> - yield cur_offset_and_size, this_data
> - cur_offset_and_size = offset_stack.next()
> + this_offset = cur_offset_and_size
> + try:
> + cur_offset_and_size = offset_stack.next()
> + except StopIteration:
> + # Close the file handle as there will be no more data
> + # The handle would normally be cleaned up as this code goes
> + # out of scope, but as we are a generator, not all code
> + # will re-enter once we have consumed all the expected
> + # data. For example:
> + # zip(range(len(requests)), readv(foo, requests))
> + # Will stop because the range is done, and not run the
> + # cleanup code for the readv().
> + fp.close()
> + cur_offset_and_size = None
> + yield this_offset, this_data
> def _sort_expand_and_combine(self, offsets, upper_limit):
> """Helper for readv.
It means that when the iterator *is* fully consumed, we will reach the
end of the function, rather than exiting from the exception raised by
It also means I don't have to have a counter tracking along with the
iterator. It moves the .next() before the yield, but this is still safe.
I did make fp.close() strictly required, which is a small api change,
but shouldn't actually cause problems.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar