Add an opt-in release close-error policy for native locks

[mmashwani]

Severity: Enhancement (callers with strict cleanup contracts cannot observe native descriptor close failures after release)

Summary

UnixFileLock._release() intentionally suppresses os.close() errors after a
successful native unlock. That default is understandable for compatibility,
especially because filelock already has a regression test preserving the
FUSE/Docker bind-mount EIO case.

I would like an opt-in release cleanup policy for callers that need close
failures to be reported as cleanup debt after ownership has been released. This
is separate from native unlock failure handling: once flock(fd, LOCK_UN)
succeeds, the object can truthfully publish an unlocked state, but callers with
strict durability or cleanup contracts may still need to know that descriptor
close failed.

Environment

• package/version: filelock 3.29.7
• source: commit 1efb8932c08789deb08ad93a91d8996cc36bb9cc
• reproduced platform: macOS, local APFS filesystem with fault injection
• affected APIs: Unix native sync and async file locks
• related issue: this complements, but does not replace, the native release
  state-loss report

Reproduction

1. Run the attached filelock_release_close_error_policy.py with
   filelock==3.29.7 on a Unix-like platform.

2. Observe:

   filelock_version=3.29.7
   injected_close_error=True
   release_error=None
   lock_is_locked=False
   stored_descriptor=None
   

The script injects an OSError(EIO) from os.close() after the real close has
run. release() returns without surfacing the cleanup failure, and the lock
object reports that it is no longer locked.

Observed vs expected

Observed:

Post-unlock descriptor close failure is always suppressed by the Unix native
backend.

Expected:

The compatibility default may keep suppressing close failures, but callers can
opt into a documented policy that reports close failures as post-release cleanup
errors without falsely retaining lock ownership.

Root cause (verified in source, 3.29.7)

UnixFileLock._release()
clears lock_file_fd, unlocks with fcntl.flock(fd, LOCK_UN), and then wraps
os.close(fd) in suppress(OSError).

BaseFileLock.release()
does not provide a caller-visible cleanup-error policy.

The current behavior is deliberate. The public test
test_release_suppresses_eio_on_close
injects OSError(EIO, "Input/output error") from os.close() and asserts that
release completes.

Python documents os.close(fd) as the low-level descriptor close operation, and
the os module documents that OS-level failures raise OSError.

Suggested fix

Add a keyword-only release cleanup policy while preserving the current default.
For example:

FileLock(path, release_close_error_policy="suppress")  # current default
FileLock(path, release_close_error_policy="raise")

Suitable designs include:

1. suppress: current behavior, preserving compatibility and the existing
   FUSE/Docker bind-mount regression test.
2. raise: surface a filelock-specific cleanup exception after the lock has
   been released and the object state has committed to unlocked.
3. collect: store the cleanup exception on a documented result, callback, or
   diagnostics field without changing default control flow.

The important invariants are:

1. A failed native unlock remains a release failure and must not be confused
   with a post-unlock close failure.
2. A close failure after successful native unlock must not falsely keep
   is_locked == True.
3. Context managers should compose this policy with the dual-failure contract
   if both the protected body and cleanup fail.
4. Async wrappers should preserve the same policy as sync release.
5. The default remains backward compatible unless maintainers intentionally
   choose a breaking change.

Verification

• Keep the existing test_release_suppresses_eio_on_close behavior for the
  default policy.
• Add a release_close_error_policy="raise" test that injects OSError(EIO)
  from os.close() and verifies the cleanup exception is visible after native
  unlock succeeds.
• Verify is_locked == False after a close-only failure when the native unlock
  succeeded.
• Verify a native LOCK_UN failure still follows the transactional release
  state contract and is not downgraded to cleanup debt.
• Verify direct context-manager, acquire-proxy context-manager, and async
  behavior use the same policy.

I did not find an existing issue or PR requesting a caller-selectable native
close-error policy. The existing test records the compatibility default, not an
opt-in strict cleanup contract.

Current workaround

Callers can flush and fsync any holder metadata before release, then accept
that filelock suppresses Unix native close failures. There is no public API for
observing that suppressed close error today.

Reproduction script

filelock_release_close_error_policy.py

"""Show that UnixFileLock suppresses close errors after native unlock."""

from __future__ import annotations

import errno
import os
import sys
import tempfile
from pathlib import Path
from unittest.mock import patch

import filelock
from filelock import FileLock
from filelock import _unix as unix_backend


def main() -> int:
    if sys.platform == "win32":
        print("This reproduction targets UnixFileLock.")
        return 2

    real_close = os.close
    with tempfile.TemporaryDirectory() as directory:
        lock = FileLock(Path(directory) / "resource.lock", timeout=0)
        lock.acquire()
        fd = lock._context.lock_file_fd  # noqa: SLF001 - inspect demonstrated state
        assert fd is not None

        injected_close_error = False

        def close_with_eio(candidate_fd: int) -> None:
            nonlocal injected_close_error
            real_close(candidate_fd)
            if candidate_fd == fd:
                injected_close_error = True
                raise OSError(errno.EIO, "injected close failure")

        release_error: BaseException | None = None
        with patch.object(unix_backend.os, "close", side_effect=close_with_eio):
            try:
                lock.release()
            except BaseException as exc:  # capture what callers receive
                release_error = exc

        print(f"filelock_version={filelock.__version__}")
        print(f"injected_close_error={injected_close_error}")
        print(f"release_error={type(release_error).__name__ if release_error else None}")
        print(f"lock_is_locked={lock.is_locked}")
        print(f"stored_descriptor={lock._context.lock_file_fd!r}")  # noqa: SLF001

        reproduced = (
            injected_close_error
            and release_error is None
            and not lock.is_locked
            and lock._context.lock_file_fd is None  # noqa: SLF001
        )
        return 0 if reproduced else 1


if __name__ == "__main__":
    raise SystemExit(main())

[gaborbernat]

Retrying close(fd) after an interrupt can close a reused descriptor: if the OS already closed the original and another thread reused its number, the retry closes an unrelated resource. CPython follows the same rule. Unix keeps close cleanup suppressed after a successful unlock, and Windows commits released state after unlock with close/unlink as post-unlock cleanup (tracked under #600). Closing.

[gaborbernat]

Reopening — I closed this against a handoff whose issue-number mapping did not match this issue's actual content, so the reason I posted was wrong. Re-reviewing against the real issue text and will follow up correctly. Apologies for the noise.