diff --git a/swift/common/bufferedhttp.py b/swift/common/bufferedhttp.py index b16147c811..7a1cb75a5b 100644 --- a/swift/common/bufferedhttp.py +++ b/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): diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index df7828bd0e..3cf898138b 100644 --- a/swift/proxy/controllers/base.py +++ b/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 diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 85056cf422..ab22e7e157 100644 --- a/test/unit/proxy/test_server.py +++ b/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 = []