[MERGE] [Bug 215426] Re: MemoryError - recv for HTTP throughProxy

[Eric Holmberg]

Okay, I've done the requested changes and added a unit test (sorry, had
to use a thread to handle the server socket).  Everything tests out
great on Windows (haven't tested it on Linux yet) and I've included
tests (which are commented out) that reproduce the problem on my system.

Have a great weekend.

-Eric

> -----Original Message-----
> From: Aaron Bentley [mailto:aaron at aaronbentley.com] 
> Sent: Wednesday, April 23, 2008 9:32 PM
> To: Eric Holmberg
> Cc: bazaar at lists.canonical.com; Andrew Bennetts
> Subject: Re: [MERGE] [Bug 215426] Re: MemoryError - recv for 
> HTTP throughProxy
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Eric Holmberg wrote:
> > The updated patch is attached and I've addressed your 
> comments below.
> > Sorry about the delay -- I was on travel last week and didn't get a 
> > chance to update the patch.
> > 
> > 
> > === modified file 'bzrlib/transport/http/response.py'
> > +    def _read_limited(self, size=-1):
> 
> I don't think _read_limited needs a default value-- it's 
> meant to be called only by read, right?
> 
> 
> > +        """Reads size bytes from the _file while limiting the read
> > +        size to 100KB.
> > +        """
> 
> ^^^ Shouldn't the meaning of -1 be documented?  It's a rather 
> unfortunate, but since it's used this way in the caller, I 
> can tolerate it.
> 
> > +        buffer = StringIO()
> > +        
> > +        if size >= 0:        
> > +            # limited read - read specified size in chunks
> > +            while size > 0:
> > +                # limit size of reads as work-around for Windows XP
> > +                # MemoryError problems in recv.
> > +                # See bug: 
> http://bugs.launchpad.net/bzr/+bug/215426 for
> > +                # details
> > +                num_bytes_to_read = 
> > + min(size,self.__read_limited_buf_size)
> 
> ^^^ items in lists should always have a space after the comma.
> 
> > +                buffer.write(self._file.read(num_bytes_to_read))
> > +                size -= num_bytes_to_read
> > +        else:
> > +            # read to EOF in chunks
> > +            while True:
> > +                data = 
> self._file.read(self.__read_limited_buf_size)
> > +                buffer.write(data)
> > +                if len(data) < self.__read_limited_buf_size:
> > +                    # EOF hit
> > +                    break
> 
> ^^^ This looks an awful lot like osutils.pumpfile.  Perhaps 
> we should extend that instead?
> 
> Also, it would be nice to have tests of at least everything testable.
> Enhancing osutils.pumpfile would definitely be a nice step in 
> that direction.
> 
> bb:resubmit
> 
> Aaron
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
> 
> iD8DBQFID/8Y0F+nu1YWqI0RAk1jAJ4qwXs9O6IxvuDMEWYP8SIKDRs61gCfbDUa
> 5Y8BfCCBnqcZ3J/KlDgeMSU=
> =YvWq
> -----END PGP SIGNATURE-----
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug215426-fix-v3.patch
Type: application/octet-stream
Size: 14215 bytes
Desc: bug215426-fix-v3.patch
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080425/2bde4c77/attachment-0002.obj