diff --git a/setup.cfg b/setup.cfg index e5ded59..2fa98a9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -50,6 +50,7 @@ testing = coverage>=4 pytest>=4 pytest-cov + pytest-timeout>=1.4.2 [bdist_wheel] universal = true diff --git a/src/filelock/_soft.py b/src/filelock/_soft.py index 1261391..83cf2b1 100644 --- a/src/filelock/_soft.py +++ b/src/filelock/_soft.py @@ -1,17 +1,32 @@ import os +import sys +from errno import EACCES, EEXIST, ENOENT from ._api import BaseFileLock +from ._util import raise_on_exist_ro_file class SoftFileLock(BaseFileLock): """Simply watches the existence of the lock file.""" def _acquire(self): - open_mode = os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_TRUNC + raise_on_exist_ro_file(self._lock_file) + # first check for exists and read-only mode as the open will mask this case as EEXIST + mode = ( + os.O_WRONLY # open for writing only + | os.O_CREAT + | os.O_EXCL # together with above raise EEXIST if the file specified by filename exists + | os.O_TRUNC # truncate the file to zero byte + ) try: - fd = os.open(self._lock_file, open_mode) - except OSError: - pass + fd = os.open(self._lock_file, mode) + except OSError as exception: + if exception.errno == EEXIST: # expected if cannot lock + pass + elif exception.errno == ENOENT: # No such file or directory - parent directory is missing + raise + elif exception.errno == EACCES and sys.platform != "win32": # Permission denied - parent dir is R/O + raise # note windows does not allow you to make a folder r/o only files else: self._lock_file_fd = fd @@ -20,8 +35,7 @@ def _release(self): self._lock_file_fd = None try: os.remove(self._lock_file) - # The file is already deleted and that's what we want. - except OSError: + except OSError: # the file is already deleted and that's what we want pass diff --git a/src/filelock/_util.py b/src/filelock/_util.py new file mode 100644 index 0000000..6d9bfff --- /dev/null +++ b/src/filelock/_util.py @@ -0,0 +1,22 @@ +import os +import stat +import sys + +PermissionError = PermissionError if sys.version_info[0] == 3 else OSError + + +def raise_on_exist_ro_file(filename): + try: + file_stat = os.stat(filename) # use stat to do exists + can write to check without race condition + except OSError: + return None # swallow does not exist or other errors + + if file_stat.st_mtime != 0: # if os.stat returns but modification is zero that's an invalid os.stat - ignore it + if not (file_stat.st_mode & stat.S_IWUSR): + raise PermissionError("Permission denied: {!r}".format(filename)) + + +__all__ = [ + "raise_on_exist_ro_file", + "PermissionError", +] diff --git a/src/filelock/_windows.py b/src/filelock/_windows.py index 825956d..473a379 100644 --- a/src/filelock/_windows.py +++ b/src/filelock/_windows.py @@ -1,6 +1,8 @@ import os +from errno import ENOENT from ._api import BaseFileLock +from ._util import raise_on_exist_ro_file try: import msvcrt @@ -12,11 +14,17 @@ class WindowsFileLock(BaseFileLock): """Uses the :func:`msvcrt.locking` function to hard lock the lock file on windows systems.""" def _acquire(self): - open_mode = os.O_RDWR | os.O_CREAT | os.O_TRUNC + raise_on_exist_ro_file(self._lock_file) + mode = ( + os.O_RDWR # open for read and write + | os.O_CREAT # create file if not exists + | os.O_TRUNC # truncate file if not empty + ) try: - fd = os.open(self._lock_file, open_mode) - except OSError: - pass + fd = os.open(self._lock_file, mode) + except OSError as exception: + if exception.errno == ENOENT: # No such file or directory + raise else: try: msvcrt.locking(fd, msvcrt.LK_NBLCK, 1) diff --git a/tests/test_filelock.py b/tests/test_filelock.py index f31f77c..d03e6f2 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -3,10 +3,13 @@ import logging import sys import threading +from contextlib import contextmanager +from stat import S_IWGRP, S_IWOTH, S_IWUSR import pytest from filelock import FileLock, SoftFileLock, Timeout +from filelock._util import PermissionError @pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock]) @@ -29,6 +32,52 @@ def test_simple(lock_type, tmp_path, caplog): assert [r.levelno for r in caplog.records] == [logging.DEBUG, logging.DEBUG, logging.DEBUG, logging.DEBUG] +@contextmanager +def make_ro(path): + write = S_IWUSR | S_IWGRP | S_IWOTH + path.chmod(path.stat().st_mode & ~write) + yield + path.chmod(path.stat().st_mode | write) + + +@pytest.fixture() +def tmp_path_ro(tmp_path): + with make_ro(tmp_path): + yield tmp_path + + +@pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock]) +@pytest.mark.skipif(sys.platform == "win32", reason="Windows does not have read only folders") +def test_ro_folder(lock_type, tmp_path_ro): + lock = lock_type(str(tmp_path_ro / "a")) + with pytest.raises(PermissionError, match="Permission denied"): + lock.acquire() + + +@pytest.fixture() +def tmp_file_ro(tmp_path): + filename = tmp_path / "a" + filename.write_text("") + with make_ro(filename): + yield filename + + +@pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock]) +def test_ro_file(lock_type, tmp_file_ro): + lock = lock_type(str(tmp_file_ro)) + with pytest.raises(PermissionError, match="Permission denied"): + lock.acquire() + + +@pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock]) +def test_missing_directory(lock_type, tmp_path_ro): + lock_path = tmp_path_ro / "a" / "b" + lock = lock_type(str(lock_path)) + + with pytest.raises(OSError, match="No such file or directory:"): + lock.acquire() + + @pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock]) def test_nested_context_manager(lock_type, tmp_path): # lock is not released before the most outer with statement that locked the lock, is left @@ -93,8 +142,8 @@ def test_nested_forced_release(lock_type, tmp_path): class ExThread(threading.Thread): - def __init__(self, target): - super(ExThread, self).__init__(target=target) + def __init__(self, target, name): + super(ExThread, self).__init__(target=target, name=name) self.ex = None def run(self): @@ -106,6 +155,7 @@ def run(self): def join(self, timeout=None): super(ExThread, self).join(timeout=timeout) if self.ex is not None: + print("fail from thread {}".format(self.name)) if sys.version_info[0] == 2: wrapper_ex = self.ex[1] raise (wrapper_ex.__class__, wrapper_ex, self.ex[2]) @@ -124,7 +174,7 @@ def thread_work(): with lock: assert lock.is_locked - threads = [ExThread(target=thread_work) for _ in range(100)] + threads = [ExThread(target=thread_work, name="t{}".format(i)) for i in range(100)] for thread in threads: thread.start() for thread in threads: @@ -138,13 +188,13 @@ def test_threaded_lock_different_lock_obj(lock_type, tmp_path): # Runs multiple threads, which acquire the same lock file with a different FileLock object. When thread group 1 # acquired the lock, thread group 2 must not hold their lock. - def thread_work_one(): + def t_1(): for _ in range(1000): with lock_1: assert lock_1.is_locked assert not lock_2.is_locked - def thread_work_two(): + def t_2(): for _ in range(1000): with lock_2: assert not lock_1.is_locked @@ -152,7 +202,7 @@ def thread_work_two(): lock_path = tmp_path / "a" lock_1, lock_2 = lock_type(str(lock_path)), lock_type(str(lock_path)) - threads = [(ExThread(target=thread_work_one), ExThread(target=thread_work_two)) for i in range(10)] + threads = [(ExThread(t_1, "t1_{}".format(i)), ExThread(t_2, "t2_{}".format(i))) for i in range(10)] for thread_1, thread_2 in threads: thread_1.start() diff --git a/tox.ini b/tox.ini index 0f126bd..7378a70 100644 --- a/tox.ini +++ b/tox.ini @@ -113,3 +113,6 @@ max-line-length = 120 [pep8] max-line-length = 120 + +[pytest] +timeout = 120 diff --git a/whitelist.txt b/whitelist.txt index b270732..029f06a 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -3,12 +3,18 @@ autodoc autosectionlabel caplog creat +eacces +eexist +enoent exc fcntl filelock fmt intersphinx intervall +iwgrp +iwoth +iwusr levelno lk lockfile @@ -18,10 +24,12 @@ nitpicky param pygments rdwr +ro skipif src tmp trunc typehints unlck +util wronly