From 739e9789c4a3d494a85bd657d808daf0c243b429 Mon Sep 17 00:00:00 2001 From: Joseph Walton Date: Sat, 5 Sep 2015 22:46:22 +1000 Subject: [PATCH 1/2] Bound how long to wait for lock files in FileCache. The file lock in FileCache is only held while writing a new version of the response. Waiting too long could be indicative of a stale lock, so default to a reasonable timeout, but also allow 'lock_timeout' to be specified on construction. --- cachecontrol/caches/file_cache.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cachecontrol/caches/file_cache.py b/cachecontrol/caches/file_cache.py index 504ad1e6..97aec6c7 100644 --- a/cachecontrol/caches/file_cache.py +++ b/cachecontrol/caches/file_cache.py @@ -50,7 +50,8 @@ def _secure_open_write(filename, fmode): class FileCache(BaseCache): def __init__(self, directory, forever=False, filemode=0o0600, - dirmode=0o0700, use_dir_lock=None, lock_class=None): + dirmode=0o0700, use_dir_lock=None, lock_class=None, + lock_timeout=30): if use_dir_lock is not None and lock_class is not None: raise ValueError("Cannot use use_dir_lock and lock_class together") @@ -66,6 +67,7 @@ def __init__(self, directory, forever=False, filemode=0o0600, self.filemode = filemode self.dirmode = dirmode self.lock_class = lock_class + self.lock_timeout = lock_timeout @staticmethod @@ -96,7 +98,7 @@ def set(self, key, value): except (IOError, OSError): pass - with self.lock_class(name) as lock: + with self.lock_class(name, timeout=self.lock_timeout) as lock: # Write our actual file with _secure_open_write(lock.path, self.filemode) as fh: fh.write(value) From 510990506a32d4c4bedf3f34079120f8c6af424d Mon Sep 17 00:00:00 2001 From: Joseph Walton Date: Mon, 30 Nov 2015 16:52:05 +1100 Subject: [PATCH 2/2] Add tests for lock_timeout. Ensure that lock_timeout defaults to a known value, which is passed through to the lock constructor, but can be overridden --- tests/test_storage_filecache.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_storage_filecache.py b/tests/test_storage_filecache.py index ec602407..708d96fe 100644 --- a/tests/test_storage_filecache.py +++ b/tests/test_storage_filecache.py @@ -8,6 +8,7 @@ import pytest import requests +from mock import ANY, Mock from cachecontrol import CacheControl from cachecontrol.caches import FileCache from lockfile import LockFile @@ -110,3 +111,20 @@ def test_lock_class(self, tmpdir): lock_class = object() cache = FileCache(str(tmpdir), lock_class=lock_class) assert cache.lock_class is lock_class + + def test_default_lock_timeout_is_passed_to_lock(self, tmpdir): + lock = Mock(__enter__=Mock(), __exit__=Mock()) + lock_class = Mock(return_value=lock) + + self.cache = FileCache(str(tmpdir), lock_class=lock_class) + self.cache.set('key', b'value') + lock_class.assert_called_once_with(ANY, timeout=30) + + def test_lock_timeout_of_none_is_passed_to_lock(self, tmpdir): + lock = Mock(__enter__=Mock(), __exit__=Mock()) + lock_class = Mock(return_value=lock) + + self.cache = FileCache(str(tmpdir), lock_class=lock_class, + lock_timeout=None) + self.cache.set('key', b'value') + lock_class.assert_called_once_with(ANY, timeout=None)