Windows: fix daemon wedge from blocking flock in hippo lock conversion#49
Open
danielhertz1999-bit wants to merge 2 commits into
Open
Conversation
downgrade_to_shared() called a *blocking* flock(base_fd, LOCK_SH) -- the only lock site in _db.py without LOCK_NB. On Windows _filelock.py services LOCK_SH as an exclusive msvcrt byte-range lock with no atomic EX->SH conversion (documented in its own module docstring), so a blocking LOCK_SH on a fd that already holds LOCK_EX polls LK_NBLCK forever against the daemon's own range. That loop runs under _PROCESS_LOCKS_GUARD, stalling every thread until the liveness watchdog force-kills the daemon -- the recurring pre_kill_forensic_dump at _db.py downgrade_to_shared. The daemon downgrades on its first WAKE tick, so this wedged the daemon whenever it genuinely held EXCLUSIVE then (intermittent: no-ops out early when the _PROCESS_LOCKS entry is already absent). Fix, per the "conversion-free protocol" the shim docstring prescribes: - downgrade_to_shared: use LOCK_SH | LOCK_NB and treat EAGAIN/EWOULDBLOCK as success. On Windows SH==EX so the fd already owns the lock (relabel only); on POSIX an EX->SH downgrade never blocks, so behavior is unchanged. - escalate_to_exclusive: same self-conflict (SH->EX on an already-held fd) was bounded by the 4s deadline so it degraded to HippoLockHeldError rather than wedging, silently running sleep consolidation without the EXCLUSIVE label on Windows. Expose SHARED_IS_EXCLUSIVE from _filelock (True on Windows, False on POSIX) and short-circuit to relabel when this fd already holds the lock. POSIX shared locks stay genuinely contended. Verified with a timed before/after: old blocking path still wedged after 3s; patched full cycle (EXCLUSIVE -> downgrade -> escalate -> downgrade) completes in 0ms with correct access-mode transitions. Existing lock/concurrency/consolidation tests unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Guards the fix in 1a2b528. Runs the exact daemon sequence (EXCLUSIVE -> downgrade -> escalate -> downgrade) with every lock call -- including close() teardown -- bounded by a watchdog-thread join timeout, so a reintroduced blocking flock FAILS the assertion within the timeout instead of hanging the suite (a wedged thread holds _PROCESS_LOCKS_GUARD, which would otherwise deadlock an un-bounded close()). Crucially these tests do NOT skip on Windows -- unlike test_hippo_concurrency.py, which is skipif(Windows) and therefore never exercised the platform where this wedge lived. That skip is exactly why the bug went uncaught. Verified: passes on the fix; on the pre-fix blocking-flock code the downgrade test fails with "the blocking-flock wedge is back" (thread never returns). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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
On Windows the daemon periodically wedges: the liveness watchdog fires
pre_kill_forensic_dump+ kill, and the blackbox thread-dump shows the main thread stuck inhippo/_db.py … downgrade_to_shared→_filelock.py … flock. This has caused real capture outages (the daemon comes back only after a manual zombie-tree kill + restart).Root cause
HippoDB.downgrade_to_sharedacquired the base lock with a blockingflock(base_fd, LOCK_SH)— the only lock site in_db.pywithoutLOCK_NB(compare the shared-acquire andescalate_to_exclusivepaths, both non-blocking + deadline-bounded).On Windows,
_filelock.pyhas no shared locks, so it servicesLOCK_SHas an exclusivemsvcrtbyte-range lock and has no atomic EX→SH conversion (its own docstring flags this hazard). A blockingLOCK_SHissued on an fd that already holdsLOCK_EXmakes the shim pollLK_NBLCKforever against the daemon's own range — Windows won't re-lock an already-locked region — so every thread stalls under the process-locks guard until the watchdog kills the process.The daemon boots holding EXCLUSIVE and downgrades on its first WAKE tick, so this fires whenever it genuinely holds EXCLUSIVE at that point (intermittent: it no-ops early when the lock entry is already absent).
Fix
downgrade_to_shared: useflock(base_fd, LOCK_SH | LOCK_NB)and treatEAGAIN/EWOULDBLOCKas success. On WindowsSH == EX, so the fd already owns the lock (relabel-only, no OS re-acquire); on POSIX an EX→SH downgrade never blocks anyway, so behavior is identical.escalate_to_exclusive(SH→EX on an already-held fd) — bounded by a 4s deadline so it degraded (raisedHippoLockHeldError) rather than wedging, but still wrong on Windows. Fixed by exposingSHARED_IS_EXCLUSIVEfrom_filelock.py(True on Windows, False on POSIX) and short-circuiting to relabel-only when this fd already holds the lock. POSIX shared locks stay genuinely contended.Test
tests/test_hippo_lock_cycle_no_wedge.pyruns the full daemon lock cycle (EXCLUSIVE → downgrade → escalate → downgrade), with every lock call — includingclose()— bounded by a watchdog-thread join timeout, so a reintroduced blocking flock fails the assertion instead of hanging the suite. It deliberately does not skip on Windows (unliketest_hippo_concurrency.py), which is exactly why this wedge went uncaught. Verified both ways: passes on the fix; on pre-fix code the downgrade test fails with the blocking-flock wedge.Windows-only behavior change; POSIX paths are byte-for-byte equivalent.