From 24237ea8a128d145d00397bd79eba06db471bcdb Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 5 May 2008 21:53:45 +0000 Subject: [PATCH] Fix a bug introduced in r62627. see issue2760 and issue2632. An assertion in readline() would fail as data was already in the internal buffer even though the socket was in unbuffered read mode. That case is now handled. More importantly, read() has been fixed to not over-recv() and leave newly recv()d data in the _fileobject buffer. The max() vs min() issue in read() is now gone. Neither was correct. On bounded reads, always ask recv() for the exact amount of data we still need. Candidate for backporting to release25-maint along with r62627. --- Lib/socket.py | 37 ++++++++++++++++++------------------- Lib/test/test_socket.py | 27 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/Lib/socket.py b/Lib/socket.py index f778f3be4d..2a5254730c 100644 --- a/Lib/socket.py +++ b/Lib/socket.py @@ -312,7 +312,8 @@ class _fileobject(object): def read(self, size=-1): # Use max, disallow tiny reads in a loop as they are very inefficient. - # We never leave read() with any leftover data in our internal buffer. + # We never leave read() with any leftover data from a new recv() call + # in our internal buffer. rbufsize = max(self._rbufsize, self.default_bufsize) # Our use of StringIO rather than lists of string objects returned by # recv() minimizes memory usage and fragmentation that occurs when @@ -342,13 +343,12 @@ class _fileobject(object): self._rbuf = StringIO() # reset _rbuf. we consume it via buf. while True: left = size - buf_len - # Using max() here means that recv() can malloc a - # large amount of memory even though recv may return - # much less data than that. But the returned data - # string is short lived in that case as we copy it - # into a StringIO and free it. - recv_size = max(rbufsize, left) - data = self._sock.recv(recv_size) + # recv() will malloc the amount of memory given as its + # parameter even though it often returns much less data + # than that. The returned data string is short lived + # as we copy it into a StringIO and free it. This avoids + # fragmentation issues on many platforms. + data = self._sock.recv(left) if not data: break n = len(data) @@ -359,13 +359,11 @@ class _fileobject(object): # - Our call to recv returned exactly the # number of bytes we were asked to read. return data - if n >= left: - # avoids data copy of: buf.write(data[:left]) - buf.write(buffer(data, 0, left)) - # avoids data copy of: self._rbuf.write(data[left:]) - self._rbuf.write(buffer(data, left)) + if n == left: + buf.write(data) del data # explicit free break + assert n <= left, "recv(%d) returned %d bytes" % (left, n) buf.write(data) buf_len += n del data # explicit free @@ -374,8 +372,9 @@ class _fileobject(object): def readline(self, size=-1): buf = self._rbuf - if self._rbufsize > 1: - # if we're buffering, check if we already have it in our buffer + buf.seek(0, 2) # seek end + if buf.tell() > 0: + # check if we already have it in our buffer buf.seek(0) bline = buf.readline(size) if bline.endswith('\n') or len(bline) == size: @@ -383,13 +382,13 @@ class _fileobject(object): self._rbuf.write(buf.read()) return bline del bline - buf.seek(0, 2) # seek end if size < 0: # Read until \n or EOF, whichever comes first if self._rbufsize <= 1: # Speed up unbuffered case - assert buf.tell() == 0 - buffers = [] + buf.seek(0) + buffers = [buf.read()] + self._rbuf = StringIO() # reset _rbuf. we consume it via buf. data = None recv = self._sock.recv while data != "\n": @@ -399,7 +398,6 @@ class _fileobject(object): buffers.append(data) return "".join(buffers) - buf = self._rbuf buf.seek(0, 2) # seek end self._rbuf = StringIO() # reset _rbuf. we consume it via buf. while True: @@ -417,6 +415,7 @@ class _fileobject(object): return buf.getvalue() else: # Read until size bytes or \n or EOF seen, whichever comes first + buf.seek(0, 2) # seek end buf_len = buf.tell() if buf_len >= size: buf.seek(0) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index aaea042f05..48c9346e0c 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -789,6 +789,33 @@ class FileObjectClassTestCase(SocketConnectedTest): self.cli_file.write(MSG) self.cli_file.flush() + def testReadlineAfterRead(self): + a_baloo_is = self.serv_file.read(len("A baloo is")) + self.assertEqual("A baloo is", a_baloo_is) + _a_bear = self.serv_file.read(len(" a bear")) + self.assertEqual(" a bear", _a_bear) + line = self.serv_file.readline() + self.assertEqual("\n", line) + line = self.serv_file.readline() + self.assertEqual("A BALOO IS A BEAR.\n", line) + line = self.serv_file.readline() + self.assertEqual(MSG, line) + + def _testReadlineAfterRead(self): + self.cli_file.write("A baloo is a bear\n") + self.cli_file.write("A BALOO IS A BEAR.\n") + self.cli_file.write(MSG) + self.cli_file.flush() + + def testReadlineAfterReadNoNewline(self): + end_of_ = self.serv_file.read(len("End Of ")) + self.assertEqual("End Of ", end_of_) + line = self.serv_file.readline() + self.assertEqual("Line", line) + + def _testReadlineAfterReadNoNewline(self): + self.cli_file.write("End Of Line") + def testClosedAttr(self): self.assert_(not self.serv_file.closed) -- 2.40.0