🐛 fix(api): make native lock release transactional

src/filelock/_api.py

@@ -475,17 +475,25 @@ def release(self, force: bool = False) -> None:  # noqa: FBT001, FBT002
         :param force: If true, the lock counter is ignored and the lock is released in every case.
 
         """
-        if self.is_locked:
+        if not self.is_locked:
+            return
+        if not force and self._context.lock_counter > 1:
             self._context.lock_counter -= 1
+            return
 
-            if self._context.lock_counter == 0 or force:
-                lock_id, lock_filename = id(self), self.lock_file
-
-                _LOGGER.debug("Attempting to release lock %s on %s", lock_id, lock_filename)
-                self._release()
-                self._context.lock_counter = 0
-                self._drop_registry_entry()
-                _LOGGER.debug("Lock %s released on %s", lock_id, lock_filename)
+        lock_id, lock_filename = id(self), self.lock_file
+        _LOGGER.debug("Attempting to release lock %s on %s", lock_id, lock_filename)
+        try:
+            self._release()
+        except BaseException:
+            # A failure after the OS unlock (during close or unlink) still released the lock: the backend cleared
+            # its descriptor, so commit the counter and registry to released even as the cleanup error propagates.
+            # A failure that left the lock held keeps the counter so a later release can retry the OS unlock.
+            if not self.is_locked:
+                self._commit_release()
+            raise
+        self._commit_release()
+        _LOGGER.debug("Lock %s released on %s", lock_id, lock_filename)
 
     def _raise_if_would_deadlock(self, canonical: str, *, timeout: float, blocking: bool) -> None:
         """
@@ -550,6 +558,11 @@ def _drop_registry_entry(self) -> None:
         """Forget this path's holder on release so a later cross-instance acquire is not misread as a deadlock."""
         _registry.held.pop(_canonical(self.lock_file), None)
 
+    def _commit_release(self) -> None:
+        """Record the lock as fully released: reset the recursion counter and drop the deadlock-registry entry."""
+        self._context.lock_counter = 0
+        self._drop_registry_entry()
+
     def _try_break_expired_lock(self) -> None:
         """Remove the lock file if its modification time exceeds the configured :attr:`lifetime`."""
         if (lifetime := self._context.lifetime) is None:

src/filelock/_unix.py

@@ -118,9 +118,12 @@ def _fallback_to_soft_lock(self) -> None:
 
         def _release(self) -> None:
             fd = cast("int", self._context.lock_file_fd)
-            self._context.lock_file_fd = None
+            # Retain the descriptor until flock succeeds: a failed unlock leaves the kernel lock held, so is_locked
+            # must keep reporting held for a retry. Once flock commits, clear held state and close as post-unlock
+            # cleanup; a close failure (EIO on FUSE/Docker bind mounts) does not make the kernel lock held again.
             fcntl.flock(fd, fcntl.LOCK_UN)
-            with suppress(OSError):  # close can raise EIO on FUSE/Docker bind-mount filesystems
+            self._context.lock_file_fd = None
+            with suppress(OSError):
                 os.close(fd)
 
 

src/filelock/_windows.py

@@ -89,10 +89,12 @@ def _acquire(self) -> None:
 
         def _release(self) -> None:
             fd = cast("int", self._context.lock_file_fd)
-            self._context.lock_file_fd = None
+            # Retain the descriptor until the OS unlock succeeds: if msvcrt.locking raises, the byte-range lock is
+            # still held, so is_locked must keep reporting held rather than losing the fd. Only after the unlock
+            # commits do close and unlink run as post-unlock cleanup; their failure cannot make the lock held again.
             msvcrt.locking(fd, msvcrt.LK_UNLCK, 1)
+            self._context.lock_file_fd = None
             os.close(fd)
-
             with suppress(OSError):
                 Path(self.lock_file).unlink()
 

src/filelock/asyncio.py

@@ -277,17 +277,35 @@ async def release(self, force: bool = False) -> None:  # ty: ignore[invalid-meth
         :param force: If true, the lock counter is ignored and the lock is released in every case.
 
         """
-        if self.is_locked:
+        if not self.is_locked:
+            return
+        if not force and self._context.lock_counter > 1:
             self._context.lock_counter -= 1
+            return
 
-            if self._context.lock_counter == 0 or force:
-                lock_id, lock_filename = id(self), self.lock_file
-
-                _LOGGER.debug("Attempting to release lock %s on %s", lock_id, lock_filename)
-                await self._run_internal_method(self._release)
-                self._context.lock_counter = 0
-                self._drop_registry_entry()
-                _LOGGER.debug("Lock %s released on %s", lock_id, lock_filename)
+        lock_id, lock_filename = id(self), self.lock_file
+        _LOGGER.debug("Attempting to release lock %s on %s", lock_id, lock_filename)
+        # Run release as its own task and shield it: a cancellation arriving mid-release must not stop the state
+        # transition halfway. On cancellation, wait for the task to finish before letting the cancel reach the caller.
+        release_task = asyncio.ensure_future(self._run_internal_method(self._release))
+        try:
+            await asyncio.shield(release_task)
+        except asyncio.CancelledError:
+            with contextlib.suppress(Exception):
+                await release_task
+            self._commit_release_if_released()
+            raise
+        except Exception:
+            self._commit_release_if_released()
+            raise
+        self._commit_release()
+        _LOGGER.debug("Lock %s released on %s", lock_id, lock_filename)
+
+    def _commit_release_if_released(self) -> None:
+        # Commit only when the backend actually unlocked (close or unlink failed after the OS unlock). If the lock is
+        # still held, keep the counter so a later release can retry.
+        if not self.is_locked:
+            self._commit_release()
 
     async def _run_internal_method(self, method: Callable[[], Any]) -> None:
         if iscoroutinefunction(method):

tests/test_async_filelock.py

@@ -4,13 +4,21 @@
 import gc
 import logging
 import sys
+import time
 from concurrent.futures import ThreadPoolExecutor
+from errno import EIO
 from pathlib import Path, PurePath
+from typing import TYPE_CHECKING
 
 import pytest
 
 from filelock import AsyncFileLock, AsyncSoftFileLock, BaseAsyncFileLock, Timeout
 
+if TYPE_CHECKING:
+    from pytest_mock import MockerFixture
+
+_UNIX_FLOCK_ONLY = pytest.mark.skipif(sys.platform == "win32", reason="native flock semantics are Unix-only")
+
 
 @pytest.mark.parametrize("lock_type", [AsyncFileLock, AsyncSoftFileLock])
 @pytest.mark.parametrize("path_type", [str, PurePath, Path])
@@ -475,3 +483,33 @@ async def test_force_release_clears_registry(tmp_path: Path, lock_type: type[Bas
     lock2 = lock_type(lock_path)
     async with lock2:
         assert lock2.is_locked
+
+
+@_UNIX_FLOCK_ONLY
+@pytest.mark.asyncio
+async def test_release_keeps_lock_held_when_unlock_fails(tmp_path: Path, mocker: MockerFixture) -> None:
+    lock = AsyncFileLock(str(tmp_path / "a"))
+    await lock.acquire()
+    mocker.patch("filelock._unix.fcntl.flock", side_effect=[OSError(EIO, "unlock failed"), None])
+    with pytest.raises(OSError, match="unlock failed"):
+        await lock.release()
+    assert lock.is_locked
+    assert lock.lock_counter == 1
+    await lock.release()
+    assert not lock.is_locked
+
+
+@_UNIX_FLOCK_ONLY
+@pytest.mark.asyncio
+async def test_release_completes_despite_cancellation(tmp_path: Path, mocker: MockerFixture) -> None:
+    lock = AsyncFileLock(str(tmp_path / "a"))
+    await lock.acquire()
+    # A slow unlock lets the cancellation land mid-release; shield must still drive it to completion.
+    mocker.patch("filelock._unix.fcntl.flock", side_effect=lambda _fd, _op: time.sleep(0.2))
+    task = asyncio.ensure_future(lock.release())
+    await asyncio.sleep(0.05)
+    task.cancel()
+    with pytest.raises(asyncio.CancelledError):
+        await task
+    assert not lock.is_locked
+    assert lock.lock_counter == 0

tests/test_filelock.py

@@ -8,7 +8,7 @@
 import time
 from concurrent.futures import ThreadPoolExecutor
 from contextlib import contextmanager
-from errno import EAGAIN, ENOSPC, ENOSYS, EWOULDBLOCK
+from errno import EAGAIN, EIO, ENOSPC, ENOSYS, EWOULDBLOCK
 from inspect import getframeinfo, stack
 from pathlib import Path, PurePath
 from stat import S_IMODE, S_IWGRP, S_IWOTH, S_IWUSR, filemode
@@ -1395,3 +1395,75 @@ def test_windows_reparse_point_lock_file_rejected(tmp_path: Path) -> None:
         FileLock(link).acquire()
 
     assert target.read_text(encoding="utf-8") == "sensitive"
+
+
+def test_release_keeps_lock_until_final_hold(tmp_path: Path) -> None:
+    lock = FileLock(str(tmp_path / "a"))
+    lock.acquire()
+    lock.acquire()
+    lock.release()
+    assert lock.is_locked
+    assert lock.lock_counter == 1
+    lock.release()
+    assert not lock.is_locked
+    assert lock.lock_counter == 0
+
+
+def test_forced_release_drops_all_holds(tmp_path: Path) -> None:
+    lock = FileLock(str(tmp_path / "a"))
+    lock.acquire()
+    lock.acquire()
+    lock.release(force=True)
+    assert not lock.is_locked
+    assert lock.lock_counter == 0
+
+
+@_UNIX_FLOCK_ONLY
+def test_unix_release_keeps_lock_held_when_unlock_fails(tmp_path: Path, mocker: MockerFixture) -> None:
+    lock = FileLock(str(tmp_path / "a"))
+    lock.acquire()
+    mocker.patch("filelock._unix.fcntl.flock", side_effect=[OSError(EIO, "unlock failed"), None])
+    with pytest.raises(OSError, match="unlock failed"):
+        lock.release()
+    assert lock.is_locked
+    assert lock.lock_counter == 1
+    lock.release()
+    assert not lock.is_locked
+
+
+@_UNIX_FLOCK_ONLY
+def test_unix_context_exit_propagates_unlock_failure(tmp_path: Path, mocker: MockerFixture) -> None:
+    # One LOCK_EX for acquire, then one LOCK_UN per release; fail only the first unlock so __exit__ propagates it.
+    mocker.patch("filelock._unix.fcntl.flock", side_effect=[None, OSError(EIO, "unlock failed"), None])
+    lock = FileLock(str(tmp_path / "a"))
+    with pytest.raises(OSError, match="unlock failed"), lock:
+        assert lock.is_locked
+    assert lock.is_locked
+    lock.release()
+    assert not lock.is_locked
+
+
+@_WINDOWS_ONLY
+def test_windows_release_keeps_lock_held_when_unlock_fails(tmp_path: Path, mocker: MockerFixture) -> None:
+    lock = FileLock(str(tmp_path / "a"))
+    lock.acquire()
+    mocker.patch("filelock._windows.msvcrt.locking", side_effect=[OSError(EIO, "unlock failed"), None])
+    with pytest.raises(OSError, match="unlock failed"):
+        lock.release()
+    assert lock.is_locked
+    assert lock.lock_counter == 1
+    lock.release()
+    assert not lock.is_locked
+
+
+@_WINDOWS_ONLY
+def test_windows_close_failure_still_commits_release(tmp_path: Path, mocker: MockerFixture) -> None:
+    lock = FileLock(str(tmp_path / "a"))
+    lock.acquire()
+    # The OS unlock succeeds, so the lock is released; the later close failure propagates but must not leave the
+    # counter or registry believing the lock is still held.
+    mocker.patch("filelock._windows.os.close", side_effect=OSError(EIO, "close failed"))
+    with pytest.raises(OSError, match="close failed"):
+        lock.release()
+    assert not lock.is_locked
+    assert lock.lock_counter == 0