x/swiftonfile
Code Issues Proposed changes

Fix double close issue in diskfile

Browse Source 
The DiskWriter was closing the file descriptor when it finished
writing but initializing it to None.  Therefore, the DiskFile
context manager would then try to close it also.

Change-Id: I188ec814d025e28c7b89532f0502ebf1d4a20a09
Signed-off-by: Luis Pabon <lpabon@redhat.com>
Reviewed-on: http://review.gluster.org/6317
Reviewed-by: Peter Portante <pportant@redhat.com>
Reviewed-by: pushpesh sharma <psharma@redhat.com>
Tested-by: pushpesh sharma <psharma@redhat.com>
Signed-off-by: Luis Pabon <lpabon@redhat.com>
Reviewed-on: http://review.gluster.org/6487
This commit is contained in:
Luis Pabon 2013-11-20 16:31:46 -05:00
parent aa0f79755f
commit b46b3dc7f2
2 changed files with 35 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
gluster/swift/obj/diskfile.py
View File

@ -367,6 +367,14 @@ class DiskFileWriter(object):
do_fadvise64(self._fd, self._last_sync, diff)
self._last_sync = self._upload_size
def close(self):
"""
Close the file descriptor
"""
if self._fd:
do_close(self._fd)
self._fd = None
def write(self, chunk):
"""
Write a chunk of data to disk.
@ -463,10 +471,9 @@ class DiskFileWriter(object):
else:
# Success!
break
# Close here so the calling context does not have to perform this,
# which keeps all file system operations in the threadpool context.
do_close(self._fd)
self._fd = None
# Close here so the calling context does not have to perform this
# in a thread.
self.close()
def put(self, metadata):
"""
@ -966,14 +973,9 @@ class DiskFile(object):
dw = DiskFileWriter(fd, tmppath, self)
yield dw
finally:
if dw is not None:
try:
if dw._fd:
do_close(dw._fd)
except OSError:
pass
if dw._tmppath:
do_unlink(dw._tmppath)
dw.close()
if dw._tmppath:
do_unlink(dw._tmppath)
def write_metadata(self, metadata):
"""

24
test/unit/obj/test_diskfile.py
View File

@ -23,7 +23,7 @@ import tempfile
import shutil
import mock
from eventlet import tpool
from mock import patch
from mock import Mock, patch
from hashlib import md5
from copy import deepcopy
@ -35,7 +35,7 @@ from gluster.swift.common.exceptions import GlusterFileSystemOSError
import gluster.swift.common.utils
from gluster.swift.common.utils import normalize_timestamp
import gluster.swift.obj.diskfile
from gluster.swift.obj.diskfile import DiskFile, OnDiskManager
from gluster.swift.obj.diskfile import DiskFileWriter, DiskFile, OnDiskManager
from gluster.swift.common.utils import DEFAULT_UID, DEFAULT_GID, X_TYPE, \
X_OBJECT_TYPE, DIR_OBJECT
@ -94,6 +94,24 @@ class MockRenamerCalled(Exception):
def _mock_renamer(a, b):
raise MockRenamerCalled()
class TestDiskFileWriter(unittest.TestCase):
""" Tests for gluster.swift.obj.diskfile.DiskFileWriter """
def test_close(self):
dw = DiskFileWriter(100, 'a', None)
mock_close = Mock()
with patch("gluster.swift.obj.diskfile.do_close", mock_close):
# It should call do_close
self.assertEqual(100, dw._fd)
dw.close()
self.assertEqual(1, mock_close.call_count)
self.assertEqual(None, dw._fd)
# It should not call do_close since it should
# have made fd equal to None
dw.close()
self.assertEqual(None, dw._fd)
self.assertEqual(1, mock_close.call_count)
class TestDiskFile(unittest.TestCase):
""" Tests for gluster.swift.obj.diskfile """
@ -881,7 +899,7 @@ class TestDiskFile(unittest.TestCase):
assert os.path.exists(saved_tmppath)
dw.write("123")
# Closing the fd prematurely should not raise any exceptions.
os.close(dw._fd)
dw.close()
assert not os.path.exists(saved_tmppath)
def test_create_err_on_unlink(self):