Merge "Stop reading from object server when client disconnects."

swift/common/bufferedhttp.py

@@ -41,6 +41,10 @@ class BufferedHTTPResponse(HTTPResponse):
     def __init__(self, sock, debuglevel=0, strict=0,
                  method=None):          # pragma: no cover
         self.sock = sock
+        # sock is an eventlet.greenio.GreenSocket
+        # sock.fd is a socket._socketobject
+        # sock.fd._sock is a socket._socket object, which is what we want.
+        self._real_socket = sock.fd._sock
         self.fp = sock.makefile('rb')
         self.debuglevel = debuglevel
         self.strict = strict
@@ -74,9 +78,25 @@ class BufferedHTTPResponse(HTTPResponse):
             self.msg = HTTPMessage(self.fp, 0)
             self.msg.fp = None
 
+    def nuke_from_orbit(self):
+        """
+        Terminate the socket with extreme prejudice.
+
+        Closes the underlying socket regardless of whether or not anyone else
+        has references to it. Use this when you are certain that nobody else
+        you care about has a reference to this socket.
+        """
+        if self._real_socket:
+            # this is idempotent; see sock_close in Modules/socketmodule.c in
+            # the Python source for details.
+            self._real_socket.close()
+        self._real_socket = None
+        self.close()
+
     def close(self):
         HTTPResponse.close(self)
         self.sock = None
+        self._real_socket = None
 
 
 class BufferedHTTPConnection(HTTPConnection):

swift/proxy/controllers/base.py

@@ -987,17 +987,17 @@ class Controller(object):
         :param src: the response from the backend
         """
         try:
-            src.swift_conn.close()
-        except Exception:
-            pass
-        src.swift_conn = None
-        try:
-            while src.read(self.app.object_chunk_size):
-                pass
-        except Exception:
-            pass
-        try:
-            src.close()
+            # Since the backends set "Connection: close" in their response
+            # headers, the response object (src) is solely responsible for the
+            # socket. The connection object (src.swift_conn) has no references
+            # to the socket, so calling its close() method does nothing, and
+            # therefore we don't do it.
+            #
+            # Also, since calling the response's close() method might not
+            # close the underlying socket but only decrement some
+            # reference-counter, we have a special method here that really,
+            # really kills the underlying socket with a close() syscall.
+            src.nuke_from_orbit()  # it's the only way to be sure
         except Exception:
             pass
 

test/unit/proxy/test_server.py

@@ -20,7 +20,6 @@ import os
 import sys
 import unittest
 import urlparse
-import signal
 from contextlib import contextmanager, nested, closing
 from gzip import GzipFile
 from shutil import rmtree
@@ -681,41 +680,31 @@ class TestObjectController(unittest.TestCase):
             self.assertEquals(res.status_int, expected)
 
     def test_GET_newest_large_file(self):
-        calls = [0]
-
-        def handler(_junk1, _junk2):
-            calls[0] += 1
-
-        old_handler = signal.signal(signal.SIGPIPE, handler)
-        try:
-            prolis = _test_sockets[0]
-            prosrv = _test_servers[0]
-            sock = connect_tcp(('localhost', prolis.getsockname()[1]))
-            fd = sock.makefile()
-            obj = 'a' * (1024 * 1024)
-            path = '/v1/a/c/o.large'
-            fd.write('PUT %s HTTP/1.1\r\n'
-                     'Host: localhost\r\n'
-                     'Connection: close\r\n'
-                     'X-Storage-Token: t\r\n'
-                     'Content-Length: %s\r\n'
-                     'Content-Type: application/octet-stream\r\n'
-                     '\r\n%s' % (path, str(len(obj)), obj))
-            fd.flush()
-            headers = readuntil2crlfs(fd)
-            exp = 'HTTP/1.1 201'
-            self.assertEqual(headers[:len(exp)], exp)
-            req = Request.blank(path,
-                                environ={'REQUEST_METHOD': 'GET'},
-                                headers={'Content-Type':
-                                         'application/octet-stream',
-                                         'X-Newest': 'true'})
-            res = req.get_response(prosrv)
-            self.assertEqual(res.status_int, 200)
-            self.assertEqual(res.body, obj)
-            self.assertEqual(calls[0], 0)
-        finally:
-            signal.signal(signal.SIGPIPE, old_handler)
+        prolis = _test_sockets[0]
+        prosrv = _test_servers[0]
+        sock = connect_tcp(('localhost', prolis.getsockname()[1]))
+        fd = sock.makefile()
+        obj = 'a' * (1024 * 1024)
+        path = '/v1/a/c/o.large'
+        fd.write('PUT %s HTTP/1.1\r\n'
+                 'Host: localhost\r\n'
+                 'Connection: close\r\n'
+                 'X-Storage-Token: t\r\n'
+                 'Content-Length: %s\r\n'
+                 'Content-Type: application/octet-stream\r\n'
+                 '\r\n%s' % (path, str(len(obj)), obj))
+        fd.flush()
+        headers = readuntil2crlfs(fd)
+        exp = 'HTTP/1.1 201'
+        self.assertEqual(headers[:len(exp)], exp)
+        req = Request.blank(path,
+                            environ={'REQUEST_METHOD': 'GET'},
+                            headers={'Content-Type':
+                                     'application/octet-stream',
+                                     'X-Newest': 'true'})
+        res = req.get_response(prosrv)
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res.body, obj)
 
     def test_PUT_expect_header_zero_content_length(self):
         test_errors = []