WindowsFileLock reports permanent open denial as lock timeout #604

Severity: Medium (a configuration or ACL failure is misclassified as lock contention)

Summary

I found that WindowsFileLock treats every EACCES from os.open() as a
failed acquisition attempt. The polling loop then reports Timeout, even when
the denial is permanent and no competing lock holder exists.

This makes operational diagnosis difficult and can cause callers to retry a
permission failure as though the resource were busy.

For callers that treat lock acquisition as part of a fail-closed filesystem
operation, this distinction matters. A polling timeout is appropriate for a
contended byte-range lock or a transient sharing violation. It is not a good
contract for a path, ACL, or policy failure that no amount of waiting can fix.

Environment

• package/version: filelock 3.29.7
• source: commit 1efb8932c08789deb08ad93a91d8996cc36bb9cc
• platform: Windows
• reproduction method: deterministic os.open() fault injection
• affected APIs: FileLock, WindowsFileLock, async Windows wrappers

Reproduction

1. Run the attached filelock_windows_access_denied_timeout.py with
   filelock==3.29.7 on Windows.

2. The script injects PermissionError(EACCES) from the lock-file open.

3. Observe:

   filelock_version=3.29.7
   injected_exception=PermissionError
   propagated_exception=Timeout
   observed_is_timeout=True
   

Observed vs expected

Observed:

A permanent open denial is swallowed and becomes Timeout.

Expected:

File-open permission failures propagate as permission errors. Only a native
locking conflict, or a documented transient sharing condition, is classified as
contention and timeout.

Root cause (verified in source, 3.29.7)

WindowsFileLock._acquire()
uses the same EACCES classification for two different operations:

• os.open() denial at lines 73-77
• msvcrt.locking() contention at lines 79-84

The approved secure Windows implementation in PR #596 retains the same
classification by returning contention for both ERROR_ACCESS_DENIED and
ERROR_SHARING_VIOLATION.

Microsoft documents these as distinct system errors: ERROR_ACCESS_DENIED
means access is denied, while ERROR_SHARING_VIOLATION means another process is
using the file:
https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-.

Suggested fix

Separate permanent open failures from lock conflicts:

• Propagate EACCES or ERROR_ACCESS_DENIED from descriptor creation unless a
  narrower documented Windows condition proves it is transient contention.
• Continue treating EACCES from msvcrt.locking() and
  ERROR_SHARING_VIOLATION from CreateFileW() as contention.
• Preserve the original OS error code, operation, and path for permanent
  failures.
• Prefer making this classification before the polling loop records a failed
  attempt, so permanent acquisition failures do not become indistinguishable
  from a real timeout.

Verification

• Inject permanent ERROR_ACCESS_DENIED during open and assert it propagates.
• Inject ERROR_SHARING_VIOLATION and assert polling and timeout remain.
• Inject EACCES from msvcrt.locking() and assert it remains contention.
• Cover immediate timeout, finite timeout, sync, and async APIs.

Current workaround

Preflight the lock directory and file ACLs before acquisition, then interpret a
Windows timeout alongside separate permission diagnostics. A preflight is
still path-based and cannot classify the actual open atomically.

Reproduction script

"""Show that a permanent Windows open denial is reported as lock contention."""

from __future__ import annotations

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


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

    import filelock
    from filelock import FileLock, Timeout
    from filelock import _windows as windows_backend

    with tempfile.TemporaryDirectory() as directory:
        path = Path(directory) / "resource.lock"
        observed: BaseException | None = None

        denial = PermissionError(errno.EACCES, "injected permanent access denial", str(path))
        with patch.object(windows_backend.os, "open", side_effect=denial):
            try:
                FileLock(path, timeout=0).acquire()
            except BaseException as exc:  # capture the public exception classification
                observed = exc

        propagated = type(observed).__name__ if observed else None
        observed_is_timeout = isinstance(observed, Timeout)
        print(f"filelock_version={filelock.__version__}")
        print(f"injected_exception={type(denial).__name__}")
        print(f"propagated_exception={propagated}")
        print(f"observed_is_timeout={observed_is_timeout}")
        return 0 if observed_is_timeout else 1


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