fix: guard fcntl.flock(LOCK_UN) with try/except OSError in finally blocks#23873
Open
vanthinh6886 wants to merge 4 commits into
Open
fix: guard fcntl.flock(LOCK_UN) with try/except OSError in finally blocks#23873vanthinh6886 wants to merge 4 commits into
vanthinh6886 wants to merge 4 commits into
Conversation
…ocks Several finally blocks call fcntl.flock(fd, LOCK_UN) without catching OSError. If the unlock fails (e.g., fd already closed, NFS stale handle, signal interruption), the exception propagates and skips subsequent cleanup code — most critically fd.close(), which leaks the file descriptor and leaves the lock held indefinitely. The msvcrt path is already guarded in every case; this brings the POSIX fcntl path to parity. Fixed sites: - cron/scheduler.py:1809 — lock_fd.close() skipped on failure - agent/google_oauth.py:229 — OSError propagated through except ImportError - tools/environments/file_sync.py:292 — lock_fd.close() skipped on failure - agent/shell_hooks.py:627 — exception propagated from generator context
Collaborator
|
Duplicate of #16274 (which covers 5 files including shell_hooks.py, scheduler.py, file_sync.py). Also overlaps with #21719, #20529, and #23819 — all applying the same fcntl.flock(LOCK_UN) guard. This PR adds google_oauth.py as a new file but the core fix is the same pattern already addressed in the earlier PRs. |
This was referenced May 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Several
finallyblocks callfcntl.flock(fd, LOCK_UN)without catchingOSError. If the unlock fails (e.g., fd already closed, NFS stale handle, signal interruption), the exception propagates and skips subsequent cleanup code — most criticallyfd.close(), which leaks the file descriptor and leaves the lock held indefinitely.The
msvcrt(Windows) path is already guarded withtry/except OSErrorin every case, but the POSIXfcntlpath is not. This asymmetry means the bug only manifests on Linux/macOS.Fix
Wrap each
fcntl.flock(fd, LOCK_UN)call intry/except OSError: pass, matching the existing guard on themsvcrtpath.Fixed Sites
cron/scheduler.pylock_fd.close()skipped on failure → fd leak + permanent locktools/environments/file_sync.pylock_fd.close()skipped on failure → fd leak + permanent lockagent/google_oauth.pyOSErrorpropagates throughexcept ImportError, outer finally still closes fdagent/shell_hooks.pywithcontext manager, fd closed by CM, but exception propagatesBefore vs After
Tests
All 4 modified files compile cleanly (
python3 -m py_compile). The fix is purely defensive — no behavioral change whenflocksucceeds.