mirror of
https://github.com/safishamsi/graphify.git
synced 2026-07-21 23:01:45 +00:00
fix(watch): unlink .rebuild.lock on release and rewrite single PID line (#858)
The rebuild lock file accumulated concatenated PIDs across post-commit rebuilds without a separator, and was never removed when the rebuild finished. Two practical consequences for users: 1. Downstream tooling that polls for `.rebuild.lock` to disappear before doing post-rebuild work (publish scripts copying graph.html to a web root, etc.) blocked forever / until its own timeout. 2. The accumulated digit string could not be parsed by humans or tooling to find the owning PID. The `_rebuild_lock` context manager now: - Opens the lock file with `a+` so a non-acquiring caller does not truncate the existing holder's PID. - After flock acquisition, truncates and writes a single `<pid>\n` line so external readers can `kill -0 $(cat .rebuild.lock)` to check liveness. - Unlinks the lock file in the finally block (only when *we* held the lock), restoring the "signal-by-absence" convention users rely on. Four regression tests added under `tests/test_watch.py` covering the PID-with-newline payload, post-release unlink, no-accumulation across sequential acquisitions, and the non-blocking-caller-does-not-clobber invariant. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
77bb10c682
commit
2c975ee9fb
@@ -2,6 +2,10 @@
|
||||
|
||||
Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases)
|
||||
|
||||
## Unreleased
|
||||
|
||||
- Fix: `graphify-out/.rebuild.lock` no longer accumulates concatenated PIDs across post-commit rebuilds — the lock file now contains a single owning PID followed by a newline while the rebuild runs, and is unlinked when it completes so downstream tooling that polls for its absence unblocks promptly (#858)
|
||||
|
||||
## 0.7.18 (2026-05-14)
|
||||
|
||||
- Fix: `graphify update` is now idempotent — graph.json and GRAPH_REPORT.md are only rewritten when content actually changes; topology comparison short-circuits clustering entirely on unchanged graphs, eliminating residual community-count drift (#824)
|
||||
|
||||
+29
-4
@@ -19,6 +19,11 @@ def _rebuild_lock(out_dir: Path, *, blocking: bool = False):
|
||||
``blocking`` is False. Uses fcntl.flock so the lock is released
|
||||
automatically if the process is killed (no stale-lock cleanup needed).
|
||||
|
||||
While the lock is held, ``.rebuild.lock`` contains the owning PID followed
|
||||
by a newline so external pollers (publish scripts, etc.) can read it.
|
||||
On successful release the file is unlinked so downstream tooling that
|
||||
waits for the lock to clear by polling for its absence unblocks promptly.
|
||||
|
||||
Falls back to a no-op yield(True) on platforms without fcntl (Windows).
|
||||
"""
|
||||
try:
|
||||
@@ -29,7 +34,11 @@ def _rebuild_lock(out_dir: Path, *, blocking: bool = False):
|
||||
|
||||
out_dir.mkdir(parents=True, exist_ok=True)
|
||||
lock_path = out_dir / ".rebuild.lock"
|
||||
fh = open(lock_path, "a", encoding="utf-8")
|
||||
# "a+" creates the file if missing without truncating an existing holder's
|
||||
# PID payload — important because another process may have already written
|
||||
# its PID before we attempt the flock.
|
||||
fh = open(lock_path, "a+", encoding="utf-8")
|
||||
acquired = False
|
||||
try:
|
||||
flags = fcntl.LOCK_EX if blocking else (fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||
try:
|
||||
@@ -37,13 +46,29 @@ def _rebuild_lock(out_dir: Path, *, blocking: bool = False):
|
||||
except BlockingIOError:
|
||||
yield False
|
||||
return
|
||||
yield True
|
||||
finally:
|
||||
acquired = True
|
||||
# Replace any prior owner's PID with ours so external readers see a
|
||||
# single parseable line, not a digit-concatenation across rebuilds.
|
||||
try:
|
||||
fcntl.flock(fh.fileno(), fcntl.LOCK_UN)
|
||||
fh.seek(0)
|
||||
fh.truncate()
|
||||
fh.write(f"{os.getpid()}\n")
|
||||
fh.flush()
|
||||
except OSError:
|
||||
pass
|
||||
yield True
|
||||
finally:
|
||||
if acquired:
|
||||
try:
|
||||
fcntl.flock(fh.fileno(), fcntl.LOCK_UN)
|
||||
except OSError:
|
||||
pass
|
||||
fh.close()
|
||||
# Signal "rebuild done" by removing the lock file. Only the holder
|
||||
# unlinks; a non-acquiring caller leaves the existing lock in place.
|
||||
if acquired:
|
||||
with contextlib.suppress(OSError):
|
||||
lock_path.unlink()
|
||||
|
||||
|
||||
def _apply_resource_limits() -> None:
|
||||
|
||||
+57
-1
@@ -1,9 +1,11 @@
|
||||
"""Tests for watch.py - file watcher helpers (no watchdog required)."""
|
||||
import os
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
import pytest
|
||||
|
||||
from graphify.watch import _notify_only, _WATCHED_EXTENSIONS
|
||||
from graphify.watch import _notify_only, _WATCHED_EXTENSIONS, _rebuild_lock
|
||||
|
||||
|
||||
# --- _notify_only ---
|
||||
@@ -96,6 +98,60 @@ def test_watch_raises_without_watchdog(tmp_path, monkeypatch):
|
||||
watch(tmp_path)
|
||||
|
||||
|
||||
# --- _rebuild_lock (GH-858) ---
|
||||
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)")
|
||||
def test_rebuild_lock_writes_pid_with_newline(tmp_path):
|
||||
out = tmp_path / "graphify-out"
|
||||
lock_path = out / ".rebuild.lock"
|
||||
with _rebuild_lock(out) as got:
|
||||
assert got is True
|
||||
assert lock_path.exists()
|
||||
contents = lock_path.read_text(encoding="utf-8")
|
||||
assert contents == f"{os.getpid()}\n", contents
|
||||
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)")
|
||||
def test_rebuild_lock_removed_after_release(tmp_path):
|
||||
"""GH-858: lock file must be unlinked once the rebuild completes so
|
||||
downstream waiters that poll for its absence unblock promptly."""
|
||||
out = tmp_path / "graphify-out"
|
||||
lock_path = out / ".rebuild.lock"
|
||||
with _rebuild_lock(out) as got:
|
||||
assert got is True
|
||||
assert not lock_path.exists(), "lock file should be unlinked after release"
|
||||
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)")
|
||||
def test_rebuild_lock_does_not_accumulate_pids_across_runs(tmp_path):
|
||||
"""GH-858: each acquisition truncates and rewrites the PID line rather
|
||||
than appending, so the file never grows into a digit-concatenation."""
|
||||
out = tmp_path / "graphify-out"
|
||||
lock_path = out / ".rebuild.lock"
|
||||
expected = f"{os.getpid()}\n"
|
||||
for _ in range(5):
|
||||
with _rebuild_lock(out) as got:
|
||||
assert got is True
|
||||
assert lock_path.read_text(encoding="utf-8") == expected
|
||||
assert not lock_path.exists()
|
||||
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)")
|
||||
def test_rebuild_lock_non_blocking_does_not_clobber_holder(tmp_path):
|
||||
"""GH-858: a non-blocking caller that fails to acquire the lock must not
|
||||
truncate the holder's PID payload."""
|
||||
out = tmp_path / "graphify-out"
|
||||
lock_path = out / ".rebuild.lock"
|
||||
with _rebuild_lock(out) as outer:
|
||||
assert outer is True
|
||||
held_contents = lock_path.read_text(encoding="utf-8")
|
||||
with _rebuild_lock(out, blocking=False) as inner:
|
||||
assert inner is False
|
||||
# Holder's PID line must still be intact.
|
||||
assert lock_path.read_text(encoding="utf-8") == held_contents
|
||||
|
||||
|
||||
def test_rebuild_code_is_idempotent_when_cluster_ids_flap(tmp_path, monkeypatch):
|
||||
from graphify import cluster as cluster_mod
|
||||
from graphify.watch import _rebuild_code
|
||||
|
||||
Reference in New Issue
Block a user