Skip to content

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

Description

@matt-edmondson

What's wrong

RestartCoreAsync (IntervalAction/IntervalAction.cs:~153-164) only stops and waits for the old PollingTask when ShouldPoll is true:

if (shouldPoll)
{
    Stop();
    await WaitAndDiscardOutcomeAsync(PollingTask).ConfigureAwait(false);
}
lock (Lock) { ShouldPoll = true; ... start new loop ... }

After Stop(), ShouldPoll is already false, but the old loop is still inside Task.Delay(PollingInterval). The restart skips the wait, sets ShouldPoll = true and starts a second loop. When the old loop wakes up it sees ShouldPoll == true and carries on. Nothing references it any more.

Failure scenario (reproduced with a temporary MSTest)

Settings: PollingInterval = 200ms, ActionInterval = 0, FromLastStart.

Sequence: Start(), wait 50 ms, then Stop(); await RestartAsync();

Observed:

  • The old polling task was not completed, and was not the same task as the new PollingTask.
  • The action ran 20 times in 2 s, where one loop gives about 10.

Every further Stop()/Restart() pair can add another orphaned loop.

Two loops also call TryRun concurrently. TryRun reads and assigns ActionTask outside the lock, so the action can overlap itself, which is the guarantee this class exists to provide.

This is a sibling of #49. That fix serialized restarts against each other, but not a restart against a loop that Stop left winding down.

Suggested fix

In RestartCoreAsync, always Stop() and await WaitAndDiscardOutcomeAsync(PollingTask) before setting ShouldPoll = true, whatever ShouldPoll was. Optionally, move TryRun's ActionTask check-and-assign under Lock as well.

Acceptance: a test for Stop, then immediately Restart, then count executions over N polling intervals shows the rate of a single loop, and the pre-restart PollingTask has completed.

Activity

  1. matt-edmondson commented on Sep 26, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    • Category: Bug
    • Priority: High. It breaks the class's core guarantees. The action runs at a multiple of the configured rate, can overlap itself, and each Stop()/Restart() pair can leak another orphaned loop. Stop then Restart is an ordinary control sequence, and the reproduction is clean.
    • Area: IntervalAction.RestartCoreAsync, TryRun
    • Suggested assignment: IntervalAction maintainer, lifecycle/threading area
    • Duplicates: None open. The issue identifies it as a sibling of Restart()/RestartAsync() permanently fails after any Action exception instead of resuming polling #49 (restart serialization), which covered restart-vs-restart but not restart-vs-a-stopping-loop.
    • In progress: No open PR covers this.

    Notes: Fix both halves together. An unconditional stop-and-await in RestartCoreAsync removes the orphaned loop. Moving TryRun's ActionTask check-and-assign under Lock is defence in depth, so that a future lifecycle bug cannot turn back into overlapping actions.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions