← Back to branch summary

~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to bzrlib/transport/__init__.py

  • Committer: John Arbash Meinel
  • Date: 2011-04-20 14:27:19 UTC
  • mto: This revision was merged to the branch mainline in revision 5837.
  • Revision ID: john@arbash-meinel.com-20110420142719-advs1k5vztqzbrgv
https://launchpad.net/bugs/767177
Fix bug #767177. Be more agressive with file.close() calls.

Our test suite gets a number of thread leaks and failures because it happens to get async
SFTPFile.close() calls. (if an SFTPFile closes due to __del__ it is done as an async request,
while if you call SFTPFile.close() it is done as a synchronous request.)
We have a couple other cases, probably. Namely SFTPTransport.get() also does an async
prefetch of the content, so if you don't .read() you'll also leak threads that think they
are doing work that you want.

The biggest change here, though, is using a try/finally in a generator, which is not 
python2.4 compatible.

Show diffs side-by-side

added added

removed removed

Lines of Context:
bzrlib/transport/__init__.py
287
287
    #       where the biggest benefit between combining reads and
288
288
    #       and seeking is. Consider a runtime auto-tune.
289
289
    _bytes_to_read_before_seek = 0
 
290
    # By default, we won't force .close() to be synchronous for remote readv.
 
291
    # (SFTP in particular has an async .close() call.) However, when running
 
292
    # the test suite, it would be really nice to have them be synchronous, to
 
293
    # keep things clean.
 
294
    _synchronous_close = False
290
295
 
291
296
    def __init__(self, base):
292
297
        super(Transport, self).__init__()
672
677
 
673
678
        This uses _coalesce_offsets to issue larger reads and fewer seeks.
674
679
 
675
 
        :param fp: A file-like object that supports seek() and read(size)
 
680
        :param fp: A file-like object that supports seek() and read(size).
 
681
            Note that implementations are allowed to call .close() on this file
 
682
            handle, so don't trust that you can use it for other work.
676
683
        :param offsets: A list of offsets to be read from the given file.
677
684
        :return: yield (pos, data) tuples for each request
678
685
        """
689
696
 
690
697
        # Cache the results, but only until they have been fulfilled
691
698
        data_map = {}
692
 
        for c_offset in coalesced:
693
 
            # TODO: jam 20060724 it might be faster to not issue seek if
694
 
            #       we are already at the right location. This should be
695
 
            #       benchmarked.
696
 
            fp.seek(c_offset.start)
697
 
            data = fp.read(c_offset.length)
698
 
            if len(data) < c_offset.length:
699
 
                raise errors.ShortReadvError(relpath, c_offset.start,
700
 
                            c_offset.length, actual=len(data))
701
 
            for suboffset, subsize in c_offset.ranges:
702
 
                key = (c_offset.start+suboffset, subsize)
703
 
                data_map[key] = data[suboffset:suboffset+subsize]
 
699
        try:
 
700
            for c_offset in coalesced:
 
701
                # TODO: jam 20060724 it might be faster to not issue seek if
 
702
                #       we are already at the right location. This should be
 
703
                #       benchmarked.
 
704
                fp.seek(c_offset.start)
 
705
                data = fp.read(c_offset.length)
 
706
                if len(data) < c_offset.length:
 
707
                    raise errors.ShortReadvError(relpath, c_offset.start,
 
708
                                c_offset.length, actual=len(data))
 
709
                for suboffset, subsize in c_offset.ranges:
 
710
                    key = (c_offset.start+suboffset, subsize)
 
711
                    data_map[key] = data[suboffset:suboffset+subsize]
704
712
 
705
 
            # Now that we've read some data, see if we can yield anything back
706
 
            while cur_offset_and_size in data_map:
707
 
                this_data = data_map.pop(cur_offset_and_size)
708
 
                this_offset = cur_offset_and_size[0]
709
 
                try:
710
 
                    cur_offset_and_size = offset_stack.next()
711
 
                except StopIteration:
712
 
                    # Close the file handle as there will be no more data
713
 
                    # The handle would normally be cleaned up as this code goes
714
 
                    # out of scope, but as we are a generator, not all code
715
 
                    # will re-enter once we have consumed all the expected
716
 
                    # data. For example:
717
 
                    #   zip(range(len(requests)), readv(foo, requests))
718
 
                    # Will stop because the range is done, and not run the
719
 
                    # cleanup code for the readv().
720
 
                    fp.close()
721
 
                    cur_offset_and_size = None
722
 
                yield this_offset, this_data
 
713
                # Now that we've read some data, see if we can yield anything back
 
714
                while cur_offset_and_size in data_map:
 
715
                    this_data = data_map.pop(cur_offset_and_size)
 
716
                    this_offset = cur_offset_and_size[0]
 
717
                    try:
 
718
                        cur_offset_and_size = offset_stack.next()
 
719
                    except StopIteration:
 
720
                        cur_offset_and_size = None
 
721
                    yield this_offset, this_data
 
722
        finally:
 
723
            # Note that this is not python2.4 compatible as try/finally in
 
724
            # generators was added in python 2.5
 
725
            fp.close()
723
726
 
724
727
    def _sort_expand_and_combine(self, offsets, upper_limit):
725
728
        """Helper for readv.