homepage

First reported by Ralf Schmitt.
I haven't checked 2.6 or 3.0.

The relevant python-dev thread is
http://mail.python.org/pipermail/python-dev/2008-April/078613.html 

One more time: the change is wrong. It should try to recv the maximum
not the minimum of size, buffer_size. If you using a buffering of 16
bytes it will otherwise call recv 256 times when you want to read 1024
bytes. this is wrong.
However there should be an upper limit, it doesn't make sense to read
10MB from socket in one recv call (imap bug).

akuchling, I added you to the nosy list. hope that's ok.

This is apparently the same issue as #2601.

ahh. yes, same issue. should have taken a look at the bugtracker before,
would have saved me some time...

I've attached a much better patch as suggested by Guido

This patch handles the case where the caller has specified the size
argument. When size is unspecified, it should be handled as if size was
infinite. By the formula from your patch, this should be
 recv_size = min(self.max_readsize, max(self._rbufsize, left))
 (== min(self.max_readsize, inf) == self.max_readsize)
and not the current code:
 if self._rbufsize <= 1:
 recv_size = self.default_bufsize
 else:
 recv_size = self._rbufsize
 while True:
 data = self._sock.recv(recv_size)
BTW, I still think this max_readsize limit should be handled somewhere
deeper in the code. at least maybe in the _socketobject class.

At least in 2.5.2, there appear to be only two places in the standard 
library where an arbitrary and unchecked size is passed directly to 
_socketobject.recv. _fileobject.read is one such place and the other 
is in logging/config.py. In both cases, the pattern is something like
while len(data) < size:
 data = data + sock.recv(size - len(data))
Of course, the same pattern may exist in any number of instances of 
user code that performs a socket.recv, and all of these would be 
subject to the same memory thrashing problem that originally beset 
imaplib.
As such, I now agree with Ralf that the fix ultimately belongs in 
_socketobject. However, deploying a fix for 2.5.2 is probably easier 
if it's in socket.py than if it requires a recompile.

As A.M. Kuchling said in the other bug :), there is no need to fix 2.5.2
as the offending change is posterior to the release.
By the way, somewhat really ought to close either this bug or #2601,
discussing things in parallel in two different bug entries is too confusing.

However it should be fixed in release25-maint. Can anyone install curt's
patch? It does what the original fix intended to do.

Twisted fixed their problem for issue 1092502 by making recv()ed data
short lived by putting it into a StringIO for buffering.
I've attached a patch that does that for the socket module -and- gets
rid of any possibility of doing tiny size recvs in read() which is the
cause of the poor performance that this issue was filed about.
My patch also documents what _rbufsize is used for (readline) and makes
sane use of it with read.
When viewing the change, it is best to view it side by side next to the
old version rather than trying to comprehend it as a diff.
I believe this really fixes both issue 1092502 while at the same time
prevents introduction of a speed problem (this issue).

(I think the twisted issue your talking about is:
http://twistedmatrix.com/trac/ticket/1079)
Your patch still contains this code:
 recv_size = min(rbufsize, left)
 data = self._sock.recv(recv_size)
This is IMHO wrong. See my comments above.

available for an easy side by side review here:
 http://codereview.appspot.com/212
Also, yes I think you're right Ralf. With these changes I should be
able to return that to a max() within the while True: for sized reads
and things will work as desired.

I fixed the min vs max use that Ralf noted and have submitted this as
r62627. all tests pass for me but I believe it deserves a wider
audience and testing outside of just the test suite.
could those who reported the original problems (both the memory use
issue and the speed issue) run their tests with this applied?
... watching the buildbots ...
 http://codereview.appspot.com/621/diff/61/81 

I have a case which triggers the assert in readline:
 File "build/bdist.linux-x86_64/egg/setuptools/command/develop.py",
line 27, in run
 File "build/bdist.linux-x86_64/egg/setuptools/command/develop.py",
line 102, in install_for_development
 File
"build/bdist.linux-x86_64/egg/setuptools/command/easy_install.py", line
519, in process_distribution
 File "build/bdist.linux-x86_64/egg/pkg_resources.py", line 522, in resolve
 File "build/bdist.linux-x86_64/egg/pkg_resources.py", line 758, in
best_match
 File "build/bdist.linux-x86_64/egg/pkg_resources.py", line 770, in obtain
 File
"build/bdist.linux-x86_64/egg/setuptools/command/easy_install.py", line
433, in easy_install
 File "build/bdist.linux-x86_64/egg/setuptools/package_index.py", line
462, in fetch_distribution
 File "build/bdist.linux-x86_64/egg/setuptools/package_index.py", line
303, in find_packages
 File "build/bdist.linux-x86_64/egg/setuptools/package_index.py", line
617, in scan_url
 File "build/bdist.linux-x86_64/egg/setuptools/package_index.py", line
198, in process_url
 File "/home/ralf/py26/lib/python2.6/socket.py", line 326, in read
data = self._sock.recv(rbufsize)
 File "/home/ralf/py26/lib/python2.6/httplib.py", line 512, in read
return self._read_chunked(amt)
 File "/home/ralf/py26/lib/python2.6/httplib.py", line 548, in
_read_chunked
line = self.fp.readline()
 File "/home/ralf/py26/lib/python2.6/socket.py", line 391, in readline
assert buf.tell() == 0
AssertionError
this happens when I run :
easy_install 'mwlib<0.6'

Similar problem is reported in #2760.

I'm glad i put that assert in... The problem occurs due to a mixture of
fixed size reads followed by unbounded readlines on an unbuffered
_fileobject. A case that the code currently doesn't handle. I'm fixing it.

The bug introduced in r62627 has been fixed in r62744.

the above easy_install now works for me. thanks.

Bumping this down to critical for the alpha release. It should be
release blocker for the first beta.

I'm going to let the committed change bake in Python trunk for a beta
release or two before backporting it to release25-maint.

Can we get the fix for release25-maint? It will not get worse than the
current state is.

If it's fixed in 2.6 and 3.0, it shouldn't be release blocker anymore,
now should it?

Well, it's a release blocker for 2.5.3.

committed to release25-maint in r64754.