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: I3ea5ae5b30f38797d4367a65239716832e344431
Signed-off-by: Prashanth Pai <ppai@redhat.com>

swiftonfile/swift/common/utils.py

@@ -275,19 +275,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
 

test/unit/common/test_utils.py

@@ -402,6 +402,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("swiftonfile.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 == {}