Skip to content

fix: guard flock(LOCK_UN) in finally blocks against OSError#16274

Open
vominh1919 wants to merge 1 commit into
NousResearch:mainfrom
vominh1919:fix/flock-unlock-guard
Open

fix: guard flock(LOCK_UN) in finally blocks against OSError#16274
vominh1919 wants to merge 1 commit into
NousResearch:mainfrom
vominh1919:fix/flock-unlock-guard

Conversation

@vominh1919

Copy link
Copy Markdown
Contributor

Problem

Five finally blocks call fcntl.flock(fd, LOCK_UN) without a try/except guard. If the unlock raises OSError (e.g., file descriptor already closed, NFS stale handle, kernel interrupt), the subsequent fd.close() is skipped and the lock file remains open and locked indefinitely.

This is the same pattern that was already guarded for msvcrt (Windows) in every one of these files — but the fcntl (Linux/macOS) path was missed.

Affected modules

File Lock protects
agent/shell_hooks.py Shell hooks allowlist
tools/memory_tool.py Memory tool file operations
tools/environments/file_sync.py Remote sync-back serialization
hermes_cli/auth.py Auth credential store
cron/scheduler.py Cron tick scheduling

Fix

Wrap each fcntl.flock(fd, LOCK_UN) in try/except OSError: pass, matching the defensive pattern already used for msvcrt.unlock in the same blocks.

Before vs After

# BEFORE — if unlock raises, close() is never called
finally:
    fcntl.flock(fd, fcntl.LOCK_UN)   # ← can raise OSError
    fd.close()                        # ← skipped on error

# AFTER — unlock failure doesn't prevent cleanup
finally:
    try:
        fcntl.flock(fd, fcntl.LOCK_UN)
    except OSError:
        pass
    fd.close()                        # ← always runs

When fcntl.flock(fd, LOCK_UN) raises OSError in a finally block,
the subsequent fd.close() is skipped, leaving the lock file open
and the lock held indefinitely. This matches the defensive pattern
already used for msvcrt.unlock in the same blocks.

Affected modules:
- agent/shell_hooks.py: shell hooks allowlist lock
- tools/memory_tool.py: memory tool file lock
- tools/environments/file_sync.py: sync-back file lock
- hermes_cli/auth.py: auth store file lock
- cron/scheduler.py: cron tick file lock
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management tool/memory Memory tool and memory providers area/auth Authentication, OAuth, credential pools labels Apr 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #16044 — same flock(LOCK_UN) guard pattern. This PR covers 5 files (superset of #16044's 3 files and #15553's 2 files). Consider consolidating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants