From 907f7d8bdef55cc84ac5c6f287bd585c82661d45 Mon Sep 17 00:00:00 2001 From: jamilahmadzai Date: Sun, 30 Aug 2026 23:12:09 +0200 Subject: [PATCH 1/2] fix: bound POSIX installer lock waits Prevent a wedged concurrent installer from blocking every later launcher indefinitely. Keep the generated POSIX copies in sync and add kernel-lock regression coverage.\n\nFixes #413 --- .../scripts/ensure-monk-agent.sh | 14 ++- .github/workflows/install-e2e.yml | 4 + plugins/monk/scripts/ensure-monk-agent.sh | 14 ++- scripts/ensure-monk-agent.sh | 14 ++- tests/ensure-monk-agent-lock-timeout.sh | 103 ++++++++++++++++++ tests/fixtures/ensure-monk-agent-lock/flock | 43 ++++++++ 6 files changed, 186 insertions(+), 6 deletions(-) create mode 100755 tests/ensure-monk-agent-lock-timeout.sh create mode 100755 tests/fixtures/ensure-monk-agent-lock/flock diff --git a/.antigravity-plugin/scripts/ensure-monk-agent.sh b/.antigravity-plugin/scripts/ensure-monk-agent.sh index 7230d99..220122c 100755 --- a/.antigravity-plugin/scripts/ensure-monk-agent.sh +++ b/.antigravity-plugin/scripts/ensure-monk-agent.sh @@ -55,10 +55,20 @@ trap cleanup EXIT # rather than fail -- per-PID scratch paths below still keep each invocation's # download/extract isolated even without the lock. if command -v flock >/dev/null 2>&1; then + install_lock_timeout="${MONK_AGENT_INSTALL_LOCK_TIMEOUT:-60}" + case "$install_lock_timeout" in + ''|*[!0-9]*) + echo "MONK_AGENT_INSTALL_LOCK_TIMEOUT must be a non-negative integer." >&2 + exit 2 + ;; + esac exec 3>"$lock_file" if ! flock -n 3; then - echo "Another monk-agent install is in progress; waiting..." >&2 - flock 3 + echo "Another monk-agent install is in progress; waiting up to ${install_lock_timeout}s..." >&2 + if ! flock -w "$install_lock_timeout" 3; then + echo "Timed out after ${install_lock_timeout}s waiting for another monk-agent install." >&2 + exit 1 + fi fi fi diff --git a/.github/workflows/install-e2e.yml b/.github/workflows/install-e2e.yml index 1f9190b..4083bbe 100644 --- a/.github/workflows/install-e2e.yml +++ b/.github/workflows/install-e2e.yml @@ -58,6 +58,10 @@ jobs: shell: bash run: ./tests/start-monk-agent-fastpath.sh + - name: Check installer lock deadline + shell: bash + run: ./tests/ensure-monk-agent-lock-timeout.sh + - name: Check launcher readiness deadline shell: bash run: ./tests/start-monk-agent-readiness-timeout.sh diff --git a/plugins/monk/scripts/ensure-monk-agent.sh b/plugins/monk/scripts/ensure-monk-agent.sh index 7230d99..220122c 100755 --- a/plugins/monk/scripts/ensure-monk-agent.sh +++ b/plugins/monk/scripts/ensure-monk-agent.sh @@ -55,10 +55,20 @@ trap cleanup EXIT # rather than fail -- per-PID scratch paths below still keep each invocation's # download/extract isolated even without the lock. if command -v flock >/dev/null 2>&1; then + install_lock_timeout="${MONK_AGENT_INSTALL_LOCK_TIMEOUT:-60}" + case "$install_lock_timeout" in + ''|*[!0-9]*) + echo "MONK_AGENT_INSTALL_LOCK_TIMEOUT must be a non-negative integer." >&2 + exit 2 + ;; + esac exec 3>"$lock_file" if ! flock -n 3; then - echo "Another monk-agent install is in progress; waiting..." >&2 - flock 3 + echo "Another monk-agent install is in progress; waiting up to ${install_lock_timeout}s..." >&2 + if ! flock -w "$install_lock_timeout" 3; then + echo "Timed out after ${install_lock_timeout}s waiting for another monk-agent install." >&2 + exit 1 + fi fi fi diff --git a/scripts/ensure-monk-agent.sh b/scripts/ensure-monk-agent.sh index 7230d99..220122c 100755 --- a/scripts/ensure-monk-agent.sh +++ b/scripts/ensure-monk-agent.sh @@ -55,10 +55,20 @@ trap cleanup EXIT # rather than fail -- per-PID scratch paths below still keep each invocation's # download/extract isolated even without the lock. if command -v flock >/dev/null 2>&1; then + install_lock_timeout="${MONK_AGENT_INSTALL_LOCK_TIMEOUT:-60}" + case "$install_lock_timeout" in + ''|*[!0-9]*) + echo "MONK_AGENT_INSTALL_LOCK_TIMEOUT must be a non-negative integer." >&2 + exit 2 + ;; + esac exec 3>"$lock_file" if ! flock -n 3; then - echo "Another monk-agent install is in progress; waiting..." >&2 - flock 3 + echo "Another monk-agent install is in progress; waiting up to ${install_lock_timeout}s..." >&2 + if ! flock -w "$install_lock_timeout" 3; then + echo "Timed out after ${install_lock_timeout}s waiting for another monk-agent install." >&2 + exit 1 + fi fi fi diff --git a/tests/ensure-monk-agent-lock-timeout.sh b/tests/ensure-monk-agent-lock-timeout.sh new file mode 100755 index 0000000..22c050d --- /dev/null +++ b/tests/ensure-monk-agent-lock-timeout.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env sh +# Regression coverage for a wedged concurrent installer: every shipped POSIX +# bootstrap must stop waiting after the configured lock deadline. +set -eu + +repo_root="$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)" +fixture_bin="$repo_root/tests/fixtures/ensure-monk-agent-lock" +work_dir="$(mktemp -d)" +holder_pid="" +watchdog_pid="" + +cleanup() { + if [ -n "$watchdog_pid" ]; then + kill "$watchdog_pid" 2>/dev/null || true + fi + if [ -n "$holder_pid" ]; then + kill "$holder_pid" 2>/dev/null || true + fi + rm -rf "$work_dir" +} +trap cleanup EXIT HUP INT TERM + +case_no=0 +for ensure_script in \ + "$repo_root/scripts/ensure-monk-agent.sh" \ + "$repo_root/plugins/monk/scripts/ensure-monk-agent.sh" \ + "$repo_root/.antigravity-plugin/scripts/ensure-monk-agent.sh" +do + case_no=$((case_no + 1)) + case_dir="$work_dir/case-$case_no" + install_dir="$case_dir/install" + ready_file="$case_dir/holder-ready" + output_file="$case_dir/output" + timeout_marker="$case_dir/watchdog-fired" + mkdir -p "$install_dir" + + python3 - "$install_dir/.monk-agent.lock" "$ready_file" <<'PY' & +import fcntl +import pathlib +import sys +import time + +lock_path, ready_path = sys.argv[1:] +with open(lock_path, "w", encoding="utf-8") as lock_file: + fcntl.flock(lock_file, fcntl.LOCK_EX) + pathlib.Path(ready_path).touch() + time.sleep(30) +PY + holder_pid=$! + + waited=0 + while [ ! -e "$ready_file" ]; do + waited=$((waited + 1)) + if [ "$waited" -ge 50 ]; then + echo "lock holder did not become ready for $ensure_script" >&2 + exit 1 + fi + sleep 0.1 + done + + set +e + PATH="$fixture_bin:$PATH" \ + MONK_AGENT_INSTALL_DIR="$install_dir" \ + MONK_AGENT_INSTALL_LOCK_TIMEOUT=1 \ + "$ensure_script" >"$output_file" 2>&1 & + ensure_pid=$! + set -e + + ( + sleep 5 + : >"$timeout_marker" + kill "$ensure_pid" 2>/dev/null || true + ) & + watchdog_pid=$! + + set +e + wait "$ensure_pid" + status=$? + set -e + kill "$watchdog_pid" 2>/dev/null || true + wait "$watchdog_pid" 2>/dev/null || true + watchdog_pid="" + + kill "$holder_pid" 2>/dev/null || true + wait "$holder_pid" 2>/dev/null || true + holder_pid="" + + if [ -e "$timeout_marker" ]; then + echo "$ensure_script waited indefinitely for the install lock" >&2 + exit 1 + fi + if [ "$status" -eq 0 ]; then + echo "$ensure_script unexpectedly succeeded while the install lock was held" >&2 + exit 1 + fi + if ! grep -Fq "Timed out after 1s waiting for another monk-agent install." "$output_file"; then + echo "$ensure_script did not explain the bounded lock failure" >&2 + cat "$output_file" >&2 + exit 1 + fi +done + +echo "ensure-monk-agent lock-timeout tests passed." diff --git a/tests/fixtures/ensure-monk-agent-lock/flock b/tests/fixtures/ensure-monk-agent-lock/flock new file mode 100755 index 0000000..aa659bf --- /dev/null +++ b/tests/fixtures/ensure-monk-agent-lock/flock @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 +"""Small cross-platform flock(1) fixture for installer lock tests.""" + +import fcntl +import sys +import time + + +def main() -> int: + args = sys.argv[1:] + nonblocking = False + timeout = None + + if args and args[0] == "-n": + nonblocking = True + args = args[1:] + elif args and args[0] == "-w": + if len(args) < 3: + return 2 + timeout = float(args[1]) + args = args[2:] + + if len(args) != 1: + return 2 + + fd = int(args[0]) + if not nonblocking and timeout is None: + fcntl.flock(fd, fcntl.LOCK_EX) + return 0 + + deadline = time.monotonic() + (timeout or 0) + while True: + try: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + return 0 + except BlockingIOError: + if nonblocking or time.monotonic() >= deadline: + return 1 + time.sleep(0.02) + + +if __name__ == "__main__": + raise SystemExit(main()) From 4bef4fd73ee853b8352e17e23b142c4aebad4f8d Mon Sep 17 00:00:00 2001 From: jamilahmadzai Date: Sun, 30 Aug 2026 23:24:43 +0200 Subject: [PATCH 2/2] test: cover installer lock recovery --- tests/ensure-monk-agent-lock-timeout.sh | 108 +++++++++++++++----- tests/fixtures/ensure-monk-agent-lock/flock | 6 ++ 2 files changed, 91 insertions(+), 23 deletions(-) diff --git a/tests/ensure-monk-agent-lock-timeout.sh b/tests/ensure-monk-agent-lock-timeout.sh index 22c050d..43458c0 100755 --- a/tests/ensure-monk-agent-lock-timeout.sh +++ b/tests/ensure-monk-agent-lock-timeout.sh @@ -1,6 +1,6 @@ #!/usr/bin/env sh -# Regression coverage for a wedged concurrent installer: every shipped POSIX -# bootstrap must stop waiting after the configured lock deadline. +# Regression coverage for concurrent installers: every shipped POSIX bootstrap +# must stop at the lock deadline, while a lock released in time must recover. set -eu repo_root="$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)" @@ -20,43 +20,64 @@ cleanup() { } trap cleanup EXIT HUP INT TERM -case_no=0 -for ensure_script in \ - "$repo_root/scripts/ensure-monk-agent.sh" \ - "$repo_root/plugins/monk/scripts/ensure-monk-agent.sh" \ - "$repo_root/.antigravity-plugin/scripts/ensure-monk-agent.sh" -do - case_no=$((case_no + 1)) - case_dir="$work_dir/case-$case_no" - install_dir="$case_dir/install" - ready_file="$case_dir/holder-ready" - output_file="$case_dir/output" - timeout_marker="$case_dir/watchdog-fired" - mkdir -p "$install_dir" +start_lock_holder() { + lock_path="$1" + ready_path="$2" + release_path="${3:-}" - python3 - "$install_dir/.monk-agent.lock" "$ready_file" <<'PY' & + python3 - "$lock_path" "$ready_path" "$release_path" <<'PY' & import fcntl import pathlib import sys import time -lock_path, ready_path = sys.argv[1:] +lock_path, ready_path, release_path = sys.argv[1:] with open(lock_path, "w", encoding="utf-8") as lock_file: fcntl.flock(lock_file, fcntl.LOCK_EX) pathlib.Path(ready_path).touch() - time.sleep(30) + if not release_path: + time.sleep(30) + else: + deadline = time.monotonic() + 5 + while not pathlib.Path(release_path).exists(): + if time.monotonic() >= deadline: + raise TimeoutError("installer never began its bounded lock wait") + time.sleep(0.02) PY holder_pid=$! waited=0 - while [ ! -e "$ready_file" ]; do + while [ ! -e "$ready_path" ]; do waited=$((waited + 1)) if [ "$waited" -ge 50 ]; then - echo "lock holder did not become ready for $ensure_script" >&2 + echo "lock holder did not become ready for $lock_path" >&2 exit 1 fi sleep 0.1 done +} + +stop_lock_holder() { + kill "$holder_pid" 2>/dev/null || true + wait "$holder_pid" 2>/dev/null || true + holder_pid="" +} + +case_no=0 +for ensure_script in \ + "$repo_root/scripts/ensure-monk-agent.sh" \ + "$repo_root/plugins/monk/scripts/ensure-monk-agent.sh" \ + "$repo_root/.antigravity-plugin/scripts/ensure-monk-agent.sh" +do + case_no=$((case_no + 1)) + case_dir="$work_dir/case-$case_no" + install_dir="$case_dir/install" + ready_file="$case_dir/holder-ready" + output_file="$case_dir/output" + timeout_marker="$case_dir/watchdog-fired" + mkdir -p "$install_dir" + + start_lock_holder "$install_dir/.monk-agent.lock" "$ready_file" set +e PATH="$fixture_bin:$PATH" \ @@ -81,9 +102,7 @@ PY wait "$watchdog_pid" 2>/dev/null || true watchdog_pid="" - kill "$holder_pid" 2>/dev/null || true - wait "$holder_pid" 2>/dev/null || true - holder_pid="" + stop_lock_holder if [ -e "$timeout_marker" ]; then echo "$ensure_script waited indefinitely for the install lock" >&2 @@ -100,4 +119,47 @@ PY fi done +# Also prove that the bounded wait does not reject an ordinary concurrent +# install that finishes before the deadline. A pre-existing executable keeps +# this path network-free after the lock is acquired. +recovery_dir="$work_dir/recovery" +install_dir="$recovery_dir/install" +ready_file="$recovery_dir/holder-ready" +wait_started="$recovery_dir/wait-started" +stdout_file="$recovery_dir/stdout" +stderr_file="$recovery_dir/stderr" +target="$install_dir/monk-agent" +mkdir -p "$install_dir" +printf '#!/usr/bin/env sh\nexit 0\n' >"$target" +chmod +x "$target" + +start_lock_holder "$install_dir/.monk-agent.lock" "$ready_file" "$wait_started" + +set +e +PATH="$fixture_bin:$PATH" \ +MONK_AGENT_INSTALL_DIR="$install_dir" \ +MONK_AGENT_INSTALL_LOCK_TIMEOUT=3 \ +MONK_AGENT_AUTO_UPDATE=0 \ +MONK_TEST_FLOCK_WAIT_STARTED="$wait_started" \ + "$repo_root/scripts/ensure-monk-agent.sh" >"$stdout_file" 2>"$stderr_file" +status=$? +set -e +stop_lock_holder + +if [ "$status" -ne 0 ]; then + echo "installer did not recover after the lock was released" >&2 + cat "$stderr_file" >&2 + exit 1 +fi +if [ "$(cat "$stdout_file")" != "$target" ]; then + echo "installer returned the wrong managed binary after lock recovery" >&2 + cat "$stdout_file" >&2 + exit 1 +fi +if ! grep -Fq "Another monk-agent install is in progress; waiting up to 3s..." "$stderr_file"; then + echo "installer did not report the bounded lock wait before recovery" >&2 + cat "$stderr_file" >&2 + exit 1 +fi + echo "ensure-monk-agent lock-timeout tests passed." diff --git a/tests/fixtures/ensure-monk-agent-lock/flock b/tests/fixtures/ensure-monk-agent-lock/flock index aa659bf..b08048e 100755 --- a/tests/fixtures/ensure-monk-agent-lock/flock +++ b/tests/fixtures/ensure-monk-agent-lock/flock @@ -2,6 +2,8 @@ """Small cross-platform flock(1) fixture for installer lock tests.""" import fcntl +import os +import pathlib import sys import time @@ -24,6 +26,10 @@ def main() -> int: return 2 fd = int(args[0]) + wait_started = os.environ.get("MONK_TEST_FLOCK_WAIT_STARTED") + if timeout is not None and wait_started: + pathlib.Path(wait_started).touch() + if not nonblocking and timeout is None: fcntl.flock(fd, fcntl.LOCK_EX) return 0