[Python-checkins] r74426 - in python/trunk: Lib/socket.py Lib/test/test_socket.py Misc/NEWS Modules/socketmodule.c

gregory.p.smith python-checkins at python.org
Thu Aug 13 20:54:50 CEST 2009


Author: gregory.p.smith
Date: Thu Aug 13 20:54:50 2009
New Revision: 74426

Log:
Fix issue1628205: Socket file objects returned by socket.socket.makefile() now
properly handles EINTR within the read, readline, write & flush methods.
The socket.sendall() method now properly handles interrupted system calls.


Modified:
   python/trunk/Lib/socket.py
   python/trunk/Lib/test/test_socket.py
   python/trunk/Misc/NEWS
   python/trunk/Modules/socketmodule.c

Modified: python/trunk/Lib/socket.py
==============================================================================
--- python/trunk/Lib/socket.py	(original)
+++ python/trunk/Lib/socket.py	Thu Aug 13 20:54:50 2009
@@ -86,9 +86,11 @@
     from StringIO import StringIO
 
 try:
-    from errno import EBADF
+    import errno
 except ImportError:
-    EBADF = 9
+    errno = None
+EBADF = getattr(errno, 'EBADF', 9)
+EINTR = getattr(errno, 'EINTR', 4)
 
 __all__ = ["getfqdn", "create_connection"]
 __all__.extend(os._get_exports_list(_socket))
@@ -286,10 +288,22 @@
 
     def flush(self):
         if self._wbuf:
-            buffer = "".join(self._wbuf)
+            data = "".join(self._wbuf)
             self._wbuf = []
             self._wbuf_len = 0
-            self._sock.sendall(buffer)
+            buffer_size = max(self._rbufsize, self.default_bufsize)
+            data_size = len(data)
+            write_offset = 0
+            try:
+                while write_offset < data_size:
+                    self._sock.sendall(buffer(data, write_offset, buffer_size))
+                    write_offset += buffer_size
+            finally:
+                if write_offset < data_size:
+                    remainder = data[write_offset:]
+                    del data  # explicit free
+                    self._wbuf.append(remainder)
+                    self._wbuf_len = len(remainder)
 
     def fileno(self):
         return self._sock.fileno()
@@ -329,7 +343,12 @@
             # Read until EOF
             self._rbuf = StringIO()  # reset _rbuf.  we consume it via buf.
             while True:
-                data = self._sock.recv(rbufsize)
+                try:
+                    data = self._sock.recv(rbufsize)
+                except error, e:
+                    if e[0] == EINTR:
+                        continue
+                    raise
                 if not data:
                     break
                 buf.write(data)
@@ -353,7 +372,12 @@
                 # 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)
+                try:
+                    data = self._sock.recv(left)
+                except error, e:
+                    if e[0] == EINTR:
+                        continue
+                    raise
                 if not data:
                     break
                 n = len(data)
@@ -396,17 +420,31 @@
                 self._rbuf = StringIO()  # reset _rbuf.  we consume it via buf.
                 data = None
                 recv = self._sock.recv
-                while data != "\n":
-                    data = recv(1)
-                    if not data:
-                        break
-                    buffers.append(data)
+                while True:
+                    try:
+                        while data != "\n":
+                            data = recv(1)
+                            if not data:
+                                break
+                            buffers.append(data)
+                    except error, e:
+                        # The try..except to catch EINTR was moved outside the
+                        # recv loop to avoid the per byte overhead.
+                        if e[0] == EINTR:
+                            continue
+                        raise
+                    break
                 return "".join(buffers)
 
             buf.seek(0, 2)  # seek end
             self._rbuf = StringIO()  # reset _rbuf.  we consume it via buf.
             while True:
-                data = self._sock.recv(self._rbufsize)
+                try:
+                    data = self._sock.recv(self._rbufsize)
+                except error, e:
+                    if e[0] == EINTR:
+                        continue
+                    raise
                 if not data:
                     break
                 nl = data.find('\n')
@@ -430,7 +468,12 @@
                 return rv
             self._rbuf = StringIO()  # reset _rbuf.  we consume it via buf.
             while True:
-                data = self._sock.recv(self._rbufsize)
+                try:
+                    data = self._sock.recv(self._rbufsize)
+                except error, e:
+                    if e[0] == EINTR:
+                        continue
+                    raise
                 if not data:
                     break
                 left = size - buf_len

Modified: python/trunk/Lib/test/test_socket.py
==============================================================================
--- python/trunk/Lib/test/test_socket.py	(original)
+++ python/trunk/Lib/test/test_socket.py	Thu Aug 13 20:54:50 2009
@@ -858,6 +858,77 @@
     def _testClosedAttr(self):
         self.assertTrue(not self.cli_file.closed)
 
+
+class FileObjectInterruptedTestCase(unittest.TestCase):
+    """Test that the file object correctly handles EINTR internally."""
+
+    class MockSocket(object):
+        def __init__(self, recv_funcs=()):
+            # A generator that returns callables that we'll call for each
+            # call to recv().
+            self._recv_step = iter(recv_funcs)
+
+        def recv(self, size):
+            return self._recv_step.next()()
+
+    @staticmethod
+    def _raise_eintr():
+        raise socket.error(errno.EINTR)
+
+    def _test_readline(self, size=-1, **kwargs):
+        mock_sock = self.MockSocket(recv_funcs=[
+                lambda : "This is the first line\nAnd the sec",
+                self._raise_eintr,
+                lambda : "ond line is here\n",
+                lambda : "",
+            ])
+        fo = socket._fileobject(mock_sock, **kwargs)
+        self.assertEquals(fo.readline(size), "This is the first line\n")
+        self.assertEquals(fo.readline(size), "And the second line is here\n")
+
+    def _test_read(self, size=-1, **kwargs):
+        mock_sock = self.MockSocket(recv_funcs=[
+                lambda : "This is the first line\nAnd the sec",
+                self._raise_eintr,
+                lambda : "ond line is here\n",
+                lambda : "",
+            ])
+        fo = socket._fileobject(mock_sock, **kwargs)
+        self.assertEquals(fo.read(size), "This is the first line\n"
+                          "And the second line is here\n")
+
+    def test_default(self):
+        self._test_readline()
+        self._test_readline(size=100)
+        self._test_read()
+        self._test_read(size=100)
+
+    def test_with_1k_buffer(self):
+        self._test_readline(bufsize=1024)
+        self._test_readline(size=100, bufsize=1024)
+        self._test_read(bufsize=1024)
+        self._test_read(size=100, bufsize=1024)
+
+    def _test_readline_no_buffer(self, size=-1):
+        mock_sock = self.MockSocket(recv_funcs=[
+                lambda : "aa",
+                lambda : "\n",
+                lambda : "BB",
+                self._raise_eintr,
+                lambda : "bb",
+                lambda : "",
+            ])
+        fo = socket._fileobject(mock_sock, bufsize=0)
+        self.assertEquals(fo.readline(size), "aa\n")
+        self.assertEquals(fo.readline(size), "BBbb")
+
+    def test_no_buffer(self):
+        self._test_readline_no_buffer()
+        self._test_readline_no_buffer(size=4)
+        self._test_read(bufsize=0)
+        self._test_read(size=100, bufsize=0)
+
+
 class UnbufferedFileObjectClassTestCase(FileObjectClassTestCase):
 
     """Repeat the tests from FileObjectClassTestCase with bufsize==0.
@@ -1253,6 +1324,7 @@
     tests.extend([
         NonBlockingTCPTests,
         FileObjectClassTestCase,
+        FileObjectInterruptedTestCase,
         UnbufferedFileObjectClassTestCase,
         LineBufferedFileObjectClassTestCase,
         SmallBufferedFileObjectClassTestCase,

Modified: python/trunk/Misc/NEWS
==============================================================================
--- python/trunk/Misc/NEWS	(original)
+++ python/trunk/Misc/NEWS	Thu Aug 13 20:54:50 2009
@@ -362,6 +362,10 @@
 - Issue #4660: If a multiprocessing.JoinableQueue.put() was preempted, it was
   possible to get a spurious 'task_done() called too many times' error.
 
+- Issue #1628205: Socket file objects returned by socket.socket.makefile() now
+  properly handles EINTR within the read, readline, write & flush methods.
+  The socket.sendall() method now properly handles interrupted system calls.
+
 - Issue #6595: The Decimal constructor now allows arbitrary Unicode
   decimal digits in input, as recommended by the standard.  Previously
   it was restricted to accepting [0-9].

Modified: python/trunk/Modules/socketmodule.c
==============================================================================
--- python/trunk/Modules/socketmodule.c	(original)
+++ python/trunk/Modules/socketmodule.c	Thu Aug 13 20:54:50 2009
@@ -2736,8 +2736,21 @@
 #else
 		n = send(s->sock_fd, buf, len, flags);
 #endif
-		if (n < 0)
+		if (n < 0) {
+#ifdef EINTR
+			/* We must handle EINTR here as there is no way for
+			 * the caller to know how much was sent otherwise.  */
+			if (errno == EINTR) {
+				/* Run signal handlers.  If an exception was
+				 * raised, abort and leave this socket in
+				 * an unknown state. */
+				if (PyErr_CheckSignals())
+					return NULL;
+				continue;
+			}
+#endif
 			break;
+		}
 		buf += n;
 		len -= n;
 	} while (len > 0);


More information about the Python-checkins mailing list