Repository navigation
Stop() followed by Restart() leaves the old polling loop alive, so two loops run the action and it can execute concurrently #61
Description
Activity
Triage
- Category: Bug
- Priority: High. An ordinary
Stop()thenRestart()breaks the class's core guarantee that executions never overlap. It also doubles the firing rate and orphans a loop whose faultsRethrowExceptions()never sees. Nothing exotic is needed to hit it. Stop/restart inside one polling interval is a normal pattern, for example when toggling a feature off and on. - Area / suggested assignee:
IntervalAction.RestartCoreAsyncand the polling-loop lifetime, plusTryRun's unlockedActionTask is nullcheck-then-assign. Owner: @matt-edmondson - Duplicates / in progress: not a duplicate. It is closely related to Stop() called while a RestartAsync() is still pending is ignored, so the action keeps running after the caller stopped it #57, where a
Stop()arrives whileRestartAsync()is still pending. The issue explains the difference, but both come fromShouldPollbeing a shared flag rather than a per-loop generation. A per-loopCancellationTokenor generation counter would fix both, so consider one PR for the pair. Open PR Keep the action's stack trace when RethrowExceptions rethrows [patch] #62 (preserving the stack trace inRethrowExceptions) does not cover it. - Next step: give each polling loop its own
CancellationTokenSource, cancelled and awaited on everyStop/Restartregardless ofShouldPoll. Move theActionTaskcheck-and-assign underLock. Add both MSTest repros from the issue as regression tests.
Generated by Claude Code
Triage
This is already fixed on
main(7c16ec8), by the v1.4.2 change "wait for a stopped polling loop before a restart starts a new one":- Old loop is always stopped:
RestartCoreAsyncnow clearsShouldPolland awaits the oldPollingTaskwhether or not it was polling. A loop left in itsTask.DelayafterStop()therefore ends before the new loop starts. - No overlapping runs:
TryRunchecks and claimsActionTaskinsidelock (Lock), so two loops can't both start the action. - Covered by a test:
RestartRightAfterStopLeavesOnlyOnePollingLoopuses this issue's own repro (200 ms interval, stop and restart during the delay). It asserts the old task has completed and that execution stays within a single loop's rate. It passes onmain.
The related race in #57, where a
Stop()during a pendingRestartAsync()was ignored, is still open and is fixed in #63. Closing this one as completed.
Generated by Claude Code
- Old loop is always stopped:
What's wrong
RestartCoreAsync(IntervalAction/IntervalAction.cs~153-164) only waits for the existingPollingTaskwhenShouldPollis currentlytrue:After a plain
Stop(),ShouldPollisfalse, but the old loop is usually still insideawait Task.Delay(PollingInterval).Restart()then skips the wait, setsShouldPoll = trueand starts a second loop. When the old loop wakes up it re-readsShouldPoll, findstrue, and keeps polling. Nothing holds a reference to it any more, becausePollingTaskhas been overwritten.The result:
TryRun(). ItsActionTask is nullcheck-then-assign is not under the lock, so both can start the action at the same time. That breaks the "no overlapping executions" behaviour the class relies on.ActionInterval/PollingIntervalallow.RethrowExceptions(), which only observesPollingTask.Stop()/Restart()inside one polling interval can add another loop.Repro (reproduced with MSTest)
PollingInterval = 200 ms,ActionInterval = 0, and the action increments a counter.var old = a.PollingTask;, calla.Stop(); a.Restart();, then wait 1 s.old.IsCompleted == falseandcount == 11. Expected:oldhas finished andcountis about 5-6.A second repro checks for overlap. With
PollingInterval = 5 msand an action that tracks its own concurrency (1 ms sleep inside), run 20 × (Delay(1),Stop(),Restart()) and then wait 3 s. Observed: max concurrent executions is 2. Expected: 1.This differs from #57 (a
Stop()that arrives while aRestartAsync()is still pending). Here theStop()has completed before an ordinaryRestart().Suggested fix
RestartCoreAsync, always callStop()andawait WaitAndDiscardOutcomeAsync(PollingTask), whatever the currentShouldPollvalue is. Alternatively, give each loop its own generation counter orCancellationToken, so a stale loop exits even ifShouldPollhas since been set back totrue.Stop(); Restart();inside one polling interval must leave the previousPollingTaskcompleted and never run the action concurrently.