Anonymous avatar Anonymous committed 2729e97

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.

Comments (0)

Files changed (2)

 
     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
             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)
                     # - 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
 
     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:
                 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":
                     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:
             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)

Lib/test/test_socket.py

         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)
 
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.