Skip to content

Restart()/RestartAsync() permanently fails after any Action exception instead of resuming polling #49

Description

@matt-edmondson

What happens

IntervalAction/IntervalAction.cs, TryRun() + RestartAsync().

When the user's Action throws, TryRun() (called from the polling loop) does throw ActionTask.Exception.GetBaseException(); on the next tick. This fault propagates out of the loop's Task.Run, so PollingTask becomes Faulted and the loop exits — but ShouldPoll is never reset to false (it's only set false inside Stop()).

The documented recovery pattern is to call RethrowExceptions() to observe the failure, then call Restart()/RestartAsync() to resume. RestartAsync() begins with:

if (shouldPoll) { Stop(); await PollingTask.ConfigureAwait(false); }

Since ShouldPoll is still true after the fault, this awaits the already-Faulted PollingTask, which rethrows the same old exception from inside RestartAsync() — before ever reaching the code that sets ShouldPoll = true and starts a new polling task.

Failure scenario

  1. Action throws once.
  2. The polling loop faults; PollingTask is now Faulted with that exception.
  3. Caller calls RethrowExceptions() (observes it), then Restart()/RestartAsync() to resume.
  4. RestartAsync() re-awaits the same faulted PollingTask and rethrows the same stale exception instead of restarting polling.

Net effect: one exception in Action kills the instance forever — every subsequent Restart()/RestartAsync() call just rethrows that same stale exception rather than resuming.

Suggested fix

In RestartAsync(), await PollingTask inside a try/catch (or check PollingTask.IsFaulted/IsCanceled and swallow/observe it) before proceeding to restart, since a faulted PollingTask should be replaceable just like a normally-completed one.

Related (same method, lower likelihood)

The "read ShouldPoll → Stop() → await old PollingTask → set ShouldPoll=true → assign new PollingTask" sequence in RestartAsync() isn't atomic as a whole (each step re-takes the lock individually). Two threads calling RestartAsync() concurrently can each pass the same checks and each spawn a Task.Run polling loop, with the second assignment overwriting the PollingTask field while the first loop keeps running unreferenced — both sharing the same ActionTask field with no coordination, which can violate the class's documented "prevents overlapping executions" guarantee. Worth a look while fixing the above, though it requires concurrent Restart calls to trigger.

Suggested acceptance criteria

  • A test that: starts the loop, makes Action throw once, calls RethrowExceptions() then RestartAsync(), and asserts polling resumes (subsequent Action invocations occur) rather than rethrowing.

Activity

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

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions