Skip to content

fix(timeout): avoid false timeout marker after normal exit#102

Open
kitsuyui wants to merge 1 commit into
mainfrom
fix/timeout-marker-after-signal
Open

fix(timeout): avoid false timeout marker after normal exit#102
kitsuyui wants to merge 1 commit into
mainfrom
fix/timeout-marker-after-signal

Conversation

@kitsuyui

Copy link
Copy Markdown
Member

Summary

  • Create the timeout marker only after the watchdog successfully sends the timeout signal.
  • Move timeout helper coverage into scripts/test-run-with-timeout.sh.
  • Add a regression test for the normal-exit race between the watchdog liveness probe and timeout signal delivery.

Changes

  • Update scripts/run-with-timeout.sh so timeout classification depends on successful signal delivery.
  • Replace the inline timeout-helper checks in .github/workflows/main.yml with the shared test script.
  • Add scripts/test-run-with-timeout.sh for timeout exit, command exit, and race regression coverage.

Motivation

The watchdog previously treated a successful liveness probe as enough evidence to mark the command as timed out. If the command exited normally before the timeout signal was delivered, the wrapper could still report exit 124. This change makes the timeout marker reflect successful signal delivery instead.

Verification

  • go run mvdan.cc/sh/v3/cmd/shfmt@v3.13.1 -d scripts/*.sh
  • shellcheck scripts/*.sh
  • go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.12 -color
  • typos
  • git diff --check
  • bash -n scripts/*.sh
  • scripts/test-run-with-timeout.sh
  • bash scripts/test-run-with-timeout-signals.sh
  • Regression check: scripts/test-run-with-timeout.sh fails against the previous run-with-timeout.sh and passes on this branch.
  • Collateral check: clean

Notes

  • The process-group signal test skips on hosts without setsid; GitHub's Ubuntu runner provides setsid.

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