Skip to content

fix: warn instead of silently suppress pid_ts write failure in acquire_boilerplates_lock#54

Open
kitsuyui wants to merge 1 commit into
mainfrom
fix/audit-stale-lock-pid-ts-write-silent-failure-001
Open

fix: warn instead of silently suppress pid_ts write failure in acquire_boilerplates_lock#54
kitsuyui wants to merge 1 commit into
mainfrom
fix/audit-stale-lock-pid-ts-write-silent-failure-001

Conversation

@kitsuyui

Copy link
Copy Markdown
Member

Summary

acquire_boilerplates_lock() writes pid_ts after acquiring the lock so that waiting processes can detect a stale lock (owner died via SIGKILL). The write was previously suppressed with 2>/dev/null || true, meaning a failure (disk full, quota, permissions) would silently disable stale detection without any visible signal.

With this change, if the write fails the action logs a warning and continues. The lock is already held (the mkdir succeeded), so operation proceeds, but the degraded state is visible in the action log for diagnosis.

Change

- printf '%s %s\n' "$$" "$(date +%s)" > "${lock_dir}/pid_ts" 2>/dev/null || true
+ if ! printf '%s %s\n' "$$" "$(date +%s)" > "${lock_dir}/pid_ts"; then
+   echo "install-gibo: warning: failed to write lock metadata (${lock_dir}/pid_ts); stale lock detection will be unavailable" >&2
+ fi

Impact

  • No functional change under normal conditions.
  • On write failure: previously silent; now emits a warning to stderr and continues.
  • The polling loop's stale-detection path ([ -f "${lock_dir}/pid_ts" ]) still behaves the same — it simply won't fire if the file was never created.
  • A SIGKILL-killed lock holder with a missing pid_ts still causes a 300-second wait; this PR makes that scenario diagnosable rather than mysterious.

Test plan

  • CI passes on all platforms (ubuntu, macos, windows)
  • No change in behavior when pid_ts write succeeds (normal path)

…e_boilerplates_lock

Previously, the pid_ts write was suppressed with `2>/dev/null || true`.
If the write failed (disk full, quota, permissions), stale lock detection
would be silently disabled: the `[ -f "${lock_dir}/pid_ts" ]` guard in the
polling loop would never fire, so a SIGKILL-killed holder would cause a
full 300-second timeout with no diagnostic.

Replace the silent suppression with an explicit warning. The lock is already
held (mkdir succeeded), so we continue, but the degraded stale-detection
state is now visible in the action log.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant