diff options
author | Samuel Merritt <sam@swiftstack.com> | 2015-06-18 12:58:03 -0700 |
---|---|---|
committer | Samuel Merritt <sam@swiftstack.com> | 2015-06-18 16:12:41 -0700 |
commit | 12d8a53fffea6e4bed8ba3d502ce625f5c6710b9 (patch) | |
tree | 83c02d5ad3a957ac903af496426da7753c4fb2ef /swift/common/request_helpers.py | |
parent | 65fce49b3ba74827d4c378f128082cf99b0b5396 (diff) | |
download | swift-12d8a53fffea6e4bed8ba3d502ce625f5c6710b9.tar.gz |
Get better at closing WSGI iterables.
PEP 333 (WSGI) says: "If the iterable returned by the application has
a close() method, the server or gateway must call that method upon
completion of the current request[.]"
There's a bunch of places where we weren't doing that; some of them
matter more than others. Calling .close() can prevent a connection
leak in some cases. In others, it just provides a certain pedantic
smugness. Either way, we should do what WSGI requires.
Noteworthy goofs include:
* If a client is downloading a large object and disconnects halfway
through, a proxy -> obj connection may be leaked. In this case,
the WSGI iterable is a SegmentedIterable, which lacked a close()
method. Thus, when the WSGI server noticed the client disconnect,
it had no way of telling the SegmentedIterable about it, and so
the underlying iterable for the segment's data didn't get
closed.
Here, it seems likely (though unproven) that the object server
would time out and kill the connection, or that a
ChunkWriteTimeout would fire down in the proxy server, so the
leaked connection would eventually go away. However, a flurry of
client disconnects could leave a big pile of useless connections.
* If a conditional request receives a 304 or 412, the underlying
app_iter is not closed. This mostly affects conditional requests
for large objects.
The leaked connections were noticed by this patch's co-author, who
made the changes to SegmentedIterable. Those changes helped, but did
not completely fix, the issue. The rest of the patch is an attempt to
plug the rest of the holes.
Co-Authored-By: Romain LE DISEZ <romain.ledisez@ovh.net>
Change-Id: I168e147aae7c1728e7e3fdabb7fba6f2d747d937
Closes-Bug: #1466549
Diffstat (limited to 'swift/common/request_helpers.py')
-rw-r--r-- | swift/common/request_helpers.py | 35 |
1 files changed, 13 insertions, 22 deletions
diff --git a/swift/common/request_helpers.py b/swift/common/request_helpers.py index c9da1cb75..c7d551c30 100644 --- a/swift/common/request_helpers.py +++ b/swift/common/request_helpers.py @@ -23,7 +23,6 @@ from swob in here without creating circular imports. import hashlib import itertools import time -from contextlib import contextmanager from urllib import unquote from swift import gettext_ as _ from swift.common.storage_policy import POLICIES @@ -32,7 +31,8 @@ from swift.common.exceptions import ListingIterError, SegmentError from swift.common.http import is_success from swift.common.swob import (HTTPBadRequest, HTTPNotAcceptable, HTTPServiceUnavailable) -from swift.common.utils import split_path, validate_device_partition +from swift.common.utils import split_path, validate_device_partition, \ + close_if_possible from swift.common.wsgi import make_subrequest @@ -249,26 +249,6 @@ def copy_header_subset(from_r, to_r, condition): to_r.headers[k] = v -def close_if_possible(maybe_closable): - close_method = getattr(maybe_closable, 'close', None) - if callable(close_method): - return close_method() - - -@contextmanager -def closing_if_possible(maybe_closable): - """ - Like contextlib.closing(), but doesn't crash if the object lacks a close() - method. - - PEP 333 (WSGI) says: "If the iterable returned by the application has a - close() method, the server or gateway must call that method upon - completion of the current request[.]" This function makes that easier. - """ - yield maybe_closable - close_if_possible(maybe_closable) - - class SegmentedIterable(object): """ Iterable that returns the object contents for a large object. @@ -304,6 +284,7 @@ class SegmentedIterable(object): self.peeked_chunk = None self.app_iter = self._internal_iter() self.validated_first_segment = False + self.current_resp = None def _internal_iter(self): start_time = time.time() @@ -360,6 +341,8 @@ class SegmentedIterable(object): 'r_size': seg_resp.content_length, 's_etag': seg_etag, 's_size': seg_size}) + else: + self.current_resp = seg_resp seg_hash = hashlib.md5() for chunk in seg_resp.app_iter: @@ -431,3 +414,11 @@ class SegmentedIterable(object): return itertools.chain([pc], self.app_iter) else: return self.app_iter + + def close(self): + """ + Called when the client disconnect. Ensure that the connection to the + backend server is closed. + """ + if self.current_resp: + close_if_possible(self.current_resp.app_iter) |