wsgi: use buffered wfile

Fix the root cause of makefile().writelines() data loss.
eventlet/eventlet#295

Also wsgi.log.write can break file-object API and not return number of bytes again.
eventlet/eventlet#296

eventlet/support/__init__.py

@@ -53,20 +53,3 @@ def capture_stderr():
     finally:
         sys.stderr = original
         stream.seek(0)
-
-
-def safe_writelines(fd, to_write):
-    # Standard Python 3 writelines() is not reliable because it doesn't care if it
-    # loses data. See CPython bug report: http://bugs.python.org/issue26292
-    for item in to_write:
-        writeall(fd, item)
-
-
-if six.PY2:
-    def writeall(fd, buf):
-        fd.write(buf)
-else:
-    def writeall(fd, buf):
-        written = 0
-        while written < len(buf):
-            written += fd.write(buf[written:])

eventlet/wsgi.py

@@ -10,9 +10,9 @@
 from eventlet import greenio
 from eventlet import greenpool
 from eventlet import support
-from eventlet.support import safe_writelines, six, writeall
 from eventlet.green import BaseHTTPServer
 from eventlet.green import socket
+from eventlet.support import six
 from eventlet.support.six.moves import urllib
 
 
@@ -112,7 +112,8 @@ def send_hundred_continue_response(self):
         # Blank line
         towrite.append(b'\r\n')
 
-        safe_writelines(self.wfile, towrite)
+        self.wfile.writelines(towrite)
+        self.wfile.flush()
 
         # Reinitialize chunk_length (expect more data)
         self.chunk_length = -1
@@ -260,7 +261,7 @@ def write(self, msg, *args):
         msg = msg + '\n'
         if args:
             msg = msg % args
-        writeall(self.log, msg)
+        self.log.write(msg)
 
 
 class FileObjectForHeaders(object):
@@ -292,6 +293,11 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
     # Contrary to stdlib, it's enabled by default.
     disable_nagle_algorithm = True
 
+    # https://github.com/eventlet/eventlet/issues/295
+    # Stdlib default is 0 (unbuffered), but then `wfile.writelines()` looses data
+    # so before going back to unbuffered, remove any usage of `writelines`.
+    wbufsize = 16 << 10
+
     def setup(self):
         # overriding SocketServer.setup to correctly handle SSL.Connection objects
         conn = self.connection = self.request
@@ -323,8 +329,7 @@ def handle_one_request(self):
         try:
             self.raw_requestline = self.rfile.readline(self.server.url_length_limit)
             if len(self.raw_requestline) == self.server.url_length_limit:
-                writeall(
-                    self.wfile,
+                self.wfile.write(
                     b"HTTP/1.0 414 Request URI Too Long\r\n"
                     b"Connection: close\r\nContent-length: 0\r\n\r\n")
                 self.close_connection = 1
@@ -346,15 +351,13 @@ def handle_one_request(self):
             if not self.parse_request():
                 return
         except HeaderLineTooLong:
-            writeall(
-                self.wfile,
+            self.wfile.write(
                 b"HTTP/1.0 400 Header Line Too Long\r\n"
                 b"Connection: close\r\nContent-length: 0\r\n\r\n")
             self.close_connection = 1
             return
         except HeadersTooLarge:
-            writeall(
-                self.wfile,
+            self.wfile.write(
                 b"HTTP/1.0 400 Headers Too Large\r\n"
                 b"Connection: close\r\nContent-length: 0\r\n\r\n")
             self.close_connection = 1
@@ -367,8 +370,7 @@ def handle_one_request(self):
             try:
                 int(content_length)
             except ValueError:
-                writeall(
-                    self.wfile,
+                self.wfile.write(
                     b"HTTP/1.0 400 Bad Request\r\n"
                     b"Connection: close\r\nContent-length: 0\r\n\r\n")
                 self.close_connection = 1
@@ -398,7 +400,7 @@ def handle_one_response(self):
         length = [0]
         status_code = [200]
 
-        def write(data, _writelines=functools.partial(safe_writelines, wfile)):
+        def write(data):
             towrite = []
             if not headers_set:
                 raise AssertionError("write() before start_response()")
@@ -447,7 +449,8 @@ def write(data, _writelines=functools.partial(safe_writelines, wfile)):
                 towrite.append(six.b("%x" % (len(data),)) + b"\r\n" + data + b"\r\n")
             else:
                 towrite.append(data)
-            _writelines(towrite)
+            wfile.writelines(towrite)
+            wfile.flush()
             length[0] = length[0] + sum(map(len, towrite))
 
         def start_response(status, response_headers, exc_info=None):

tests/greenio_test.py

@@ -880,6 +880,41 @@ def test_double_close_219():
     tests.run_isolated('greenio_double_close_219.py')
 
 
+def test_partial_write_295():
+    # https://github.com/eventlet/eventlet/issues/295
+    # `socket.makefile('w').writelines()` must send all
+    # despite partial writes by underlying socket
+    listen_socket = eventlet.listen(('localhost', 0))
+    original_accept = listen_socket.accept
+
+    def talk(conn):
+        f = conn.makefile('wb')
+        line = b'*' * 2140
+        f.writelines([line] * 10000)
+        conn.close()
+
+    def accept():
+        connection, address = original_accept()
+        original_send = connection.send
+
+        def slow_send(b, *args):
+            b = b[:1031]
+            return original_send(b, *args)
+
+        connection.send = slow_send
+        eventlet.spawn(talk, connection)
+        return connection, address
+
+    listen_socket.accept = accept
+
+    eventlet.spawn(listen_socket.accept)
+    sock = eventlet.connect(listen_socket.getsockname())
+    with eventlet.Timeout(10):
+        bs = sock.makefile('rb').read()
+    assert len(bs) == 21400000
+    assert bs == (b'*' * 21400000)
+
+
 def test_socket_file_read_non_int():
     listen_socket = eventlet.listen(('localhost', 0))
 

tests/wsgi_test.py

@@ -389,17 +389,16 @@ def test_011_multiple_chunks(self):
         self.assertEqual(response, b'\r\n')
 
     def test_partial_writes_are_handled(self):
+        # https://github.com/eventlet/eventlet/issues/295
+        # Eventlet issue: "Python 3: wsgi doesn't handle correctly partial
+        # write of socket send() when using writelines()".
+        #
         # The bug was caused by the default writelines() implementaiton
         # (used by the wsgi module) which doesn't check if write()
         # successfully completed sending *all* data therefore data could be
         # lost and the client could be left hanging forever.
         #
-        # This test additionally ensures that plain write() calls in the wsgi
-        # are also correct now (replaced with writeare also correct now (replaced with writeall()).
-        #
-        # Eventlet issue: "Python 3: wsgi doesn't handle correctly partial
-        # write of socket send() when using writelines()",
-        # https://github.com/eventlet/eventlet/issues/295
+        # Switching wsgi wfile to buffered mode fixes the issue.
         #
         # Related CPython issue: "Raw I/O writelines() broken",
         # http://bugs.python.org/issue26292