summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPrashanth Pai <ppai@redhat.com>2016-03-03 17:25:48 +0530
committerThiago da Silva <thiago@redhat.com>2016-03-08 11:01:38 -0800
commit25188ca49950267a74b35aab1359bd5d3b919fc7 (patch)
treeb8b4ec27e3ddbf9a3ecf7c2ae75d28cf6bebc94b
parent7392f4626609da910a2c98c14b8380419613635d (diff)
Fix dup fd leak
A fd was not being closed after it was duplicated. This code path can be easily hit when doing a GET on a file that needs Etag (md5sum) to be recalculated. Change-Id: Ib2e10d990b9b2e1fa85d0079767892de8c8d4eec Signed-off-by: Prashanth Pai <ppai@redhat.com> Reviewed-on: http://review.gluster.org/13593 Reviewed-by: Thiago da Silva <thiago@redhat.com> Tested-by: Thiago da Silva <thiago@redhat.com>
-rw-r--r--gluster/swift/common/utils.py15
-rw-r--r--test/functional_auth/common_conf/object-server.conf2
-rw-r--r--test/unit/common/test_utils.py16
3 files changed, 27 insertions, 6 deletions
diff --git a/gluster/swift/common/utils.py b/gluster/swift/common/utils.py
index 3bcd074..ded2f0b 100644
--- a/gluster/swift/common/utils.py
+++ b/gluster/swift/common/utils.py
@@ -415,19 +415,26 @@ def _get_etag(path_or_fd):
Since we don't have that we should yield after each chunk read and
computed so that we don't consume the worker thread.
"""
+ etag = ''
if isinstance(path_or_fd, int):
# We are given a file descriptor, so this is an invocation from the
# DiskFile.open() method.
fd = path_or_fd
- etag = _read_for_etag(do_dup(fd))
- do_lseek(fd, 0, os.SEEK_SET)
+ dup_fd = do_dup(fd)
+ try:
+ etag = _read_for_etag(dup_fd)
+ do_lseek(fd, 0, os.SEEK_SET)
+ finally:
+ do_close(dup_fd)
else:
# We are given a path to the object when the DiskDir.list_objects_iter
# method invokes us.
path = path_or_fd
fd = do_open(path, os.O_RDONLY)
- etag = _read_for_etag(fd)
- do_close(fd)
+ try:
+ etag = _read_for_etag(fd)
+ finally:
+ do_close(fd)
return etag
diff --git a/test/functional_auth/common_conf/object-server.conf b/test/functional_auth/common_conf/object-server.conf
index 28076cd..2599c87 100644
--- a/test/functional_auth/common_conf/object-server.conf
+++ b/test/functional_auth/common_conf/object-server.conf
@@ -51,5 +51,3 @@ network_chunk_size = 65556
# across all conf files!
auto_create_account_prefix = gs
expiring_objects_account_name = expiring
-
-gluster_swift_mode = true
diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py
index 5ab6f36..f88229a 100644
--- a/test/unit/common/test_utils.py
+++ b/test/unit/common/test_utils.py
@@ -439,6 +439,22 @@ class TestUtils(unittest.TestCase):
md5.update(chunk)
assert hd == md5.hexdigest()
+ def test_get_etag_dup_fd_closed(self):
+ fd, path = tempfile.mkstemp()
+ data = "It's not who we are underneath, but what we do that defines us"
+ os.write(fd, data)
+ os.lseek(fd, 0, os.SEEK_SET)
+
+ mock_do_close = Mock()
+ with patch("gluster.swift.common.utils.do_close", mock_do_close):
+ etag = utils._get_etag(fd)
+ self.assertEqual(etag, hashlib.md5(data).hexdigest())
+ self.assertTrue(mock_do_close.called)
+
+ # We mocked out close, so we have to close the fd for real
+ os.close(mock_do_close.call_args[0][0])
+ os.close(fd)
+
def test_get_object_metadata_dne(self):
md = utils.get_object_metadata("/tmp/doesNotEx1st")
assert md == {}