Skip to content

Wait for a stopped polling loop before a restart starts a new one - #55

Merged
matt-edmondson merged 1 commit into
mainfrom
claude/intervalaction-52-restart-after-stop
Sep 27, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
claude/intervalaction-52-restart-after-stop

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #52

What was wrong

RestartCoreAsync stopped and awaited the old PollingTask only when ShouldPoll was still true. After Stop(), ShouldPoll is already false, but the old loop can still be inside Task.Delay(PollingInterval). The restart skipped the wait, set ShouldPoll = true and started a second loop. When the old loop woke, it saw ShouldPoll == true and kept going with nothing referencing it. The action then ran at double rate, and each later Stop()/Restart() pair could leak another loop.

Change

  • RestartCoreAsync now always calls Stop() and awaits the previous PollingTask before it sets ShouldPoll again. Stop() is idempotent, and awaiting an already-completed task is free, so the path where the loop was still polling is unchanged.
  • As defence in depth, TryRun now checks and claims ActionTask while holding Lock, as the issue triage suggested. Two callers can then never both see the slot empty and each start the action. Lock is not held while the action runs; the action task takes it on its own thread, only briefly, to set LastRunTime.

Tests

  • New test: RestartRightAfterStopLeavesOnlyOnePollingLoop. It calls Start(), waits 50 ms, then calls Stop() and immediately await RestartAsync(). It asserts that the pre-restart PollingTask has completed and has been replaced. Over 10 polling intervals, it also asserts that the action ran no more often than a single loop allows. The bound comes from a measured stopwatch rather than a fixed count, so a slow runner doesn't make it flaky.
  • With the library change reverted, the test fails because the old polling task is still running. With the change, it passes.
  • The full suite passed three times in a row locally: 15 of 15.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TX4nouJvSXNs7eSvDP13qU


Generated by Claude Code

…e [patch]

RestartCoreAsync only stopped and awaited the old PollingTask when
ShouldPoll was still true. After Stop(), the old loop could still be
inside its delay; the restart set ShouldPoll again, so the old loop woke
up and kept running beside the new one, doubling the action rate. Each
further Stop/Restart pair could add another orphaned loop.

The restart now always stops and awaits the previous loop. TryRun also
checks and claims ActionTask under the lock, so two callers can never
both start the action.

Fixes #52

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TX4nouJvSXNs7eSvDP13qU
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit 60524eb into main Sep 27, 2026
12 checks passed
@matt-edmondson
matt-edmondson deleted the claude/intervalaction-52-restart-after-stop branch September 27, 2026 15:05
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.

Restart right after Stop leaves the old polling loop alive, so two loops run the action at double rate and can overlap it

1 participant