Wait for a stopped polling loop before a restart starts a new one

IntervalAction.Test/IntervalActionTests.cs

@@ -46,7 +46,7 @@
 			{
 				Interlocked.Increment(ref executions);
 				// Simulate a long running task.
 				Thread.Sleep(500);
 			},
 			IntervalType = IntervalType.FromLastStart
 		};
@@ -149,6 +149,48 @@
 		intervalAction.RethrowExceptions();
 	}
 
+	[TestMethod]
+	public async Task RestartRightAfterStopLeavesOnlyOnePollingLoop()
+	{
+		// Arrange
+		int counter = 0;
+		TimeSpan pollingInterval = TimeSpan.FromMilliseconds(200);
+		IntervalActionOptions options = new()
+		{
+			PollingInterval = pollingInterval,
+			ActionInterval = TimeSpan.Zero,
+			Action = () => Interlocked.Increment(ref counter),
+			IntervalType = IntervalType.FromLastStart
+		};
+
+		IntervalAction intervalAction = IntervalAction.Start(options);
+		// Let the loop run its first tick and enter its delay.
+		await Task.Delay(50).ConfigureAwait(false);
+
+		// Act: stop, then restart while the old loop is still inside its delay
+		Task oldPollingTask = intervalAction.PollingTask;
+		intervalAction.Stop();
+		await intervalAction.RestartAsync().ConfigureAwait(false);
+
+		// Assert: the old loop has ended rather than being left to resume alongside the new one
+		Assert.IsTrue(oldPollingTask.IsCompleted, "Old polling task should have completed before the restart started a new one");
+		Assert.AreNotSame(oldPollingTask, intervalAction.PollingTask);
+
+		int counterAtRestart = Volatile.Read(ref counter);
+		System.Diagnostics.Stopwatch stopwatch = System.Diagnostics.Stopwatch.StartNew();
+		await Task.Delay(TimeSpan.FromTicks(pollingInterval.Ticks * 10)).ConfigureAwait(false);
+		intervalAction.Stop();
+		await intervalAction.PollingTask.ConfigureAwait(false);
+		stopwatch.Stop();
+
+		// One loop starts the action at most once per polling interval, plus its first tick
+		int executions = Volatile.Read(ref counter) - counterAtRestart;
+		int maxExecutionsForOneLoop = (int)(stopwatch.Elapsed.Ticks / pollingInterval.Ticks) + 1;
+		Assert.IsLessThanOrEqualTo(maxExecutionsForOneLoop, executions, "The action ran faster than a single polling loop allows");
+
+		intervalAction.RethrowExceptions();
+	}
+
 	[TestMethod]
 	public async Task StopPollingTaskStopsExecuting()
 	{

IntervalAction/IntervalAction.cs

@@ -150,18 +150,11 @@ private async Task RestartCoreAsync(Task previousRestart)
 	{
 		await WaitAndDiscardOutcomeAsync(previousRestart).ConfigureAwait(false);
 
-		bool shouldPoll;
-
-		lock (Lock)
-		{
-			shouldPoll = ShouldPoll;
-		}
-
-		if (shouldPoll)
-		{
-			Stop();
-			await WaitAndDiscardOutcomeAsync(PollingTask).ConfigureAwait(false);
-		}
+		// Wait for the old loop whether or not it is still meant to be polling. After Stop() it can
+		// still be inside its delay, and would see ShouldPoll set again below and keep running
+		// alongside the new loop, unreferenced.
+		Stop();
+		await WaitAndDiscardOutcomeAsync(PollingTask).ConfigureAwait(false);
 
 		lock (Lock)
 		{
@@ -208,50 +201,48 @@ internal bool TryRun()
 	{
 		Ensure.NotNull(Action);
 
-		if (ActionTask?.IsCompleted ?? false)
+		// Check and claim ActionTask under the lock, so two callers can never both see it empty and
+		// each start the action
+		lock (Lock)
 		{
-			if (ActionTask.Exception is not null)
+			if (ActionTask?.IsCompleted ?? false)
 			{
-				throw ActionTask.Exception.GetBaseException();
-			}
-
-			ActionTask = null;
-		}
-
-		DateTimeOffset lastRunTime;
+				if (ActionTask.Exception is not null)
+				{
+					throw ActionTask.Exception.GetBaseException();
+				}
 
-		lock (Lock)
-		{
-			lastRunTime = LastRunTime;
-		}
+				ActionTask = null;
+			}
 
-		if (ActionInterval >= TimeSpan.Zero && ActionTask is null && DateTimeOffset.Now - lastRunTime > ActionInterval)
-		{
-			ActionTask = Task.Run(() =>
+			if (ActionInterval >= TimeSpan.Zero && ActionTask is null && DateTimeOffset.Now - LastRunTime > ActionInterval)
 			{
-				if (IntervalType == IntervalType.FromLastStart)
+				ActionTask = Task.Run(() =>
 				{
-					lock (Lock)
+					if (IntervalType == IntervalType.FromLastStart)
 					{
-						LastRunTime = DateTimeOffset.Now;
+						lock (Lock)
+						{
+							LastRunTime = DateTimeOffset.Now;
+						}
 					}
-				}
 
-				Action();
+					Action();
 
-				if (IntervalType == IntervalType.FromLastCompletion)
-				{
-					lock (Lock)
+					if (IntervalType == IntervalType.FromLastCompletion)
 					{
-						LastRunTime = DateTimeOffset.Now;
+						lock (Lock)
+						{
+							LastRunTime = DateTimeOffset.Now;
+						}
 					}
-				}
-			});
+				});
 
-			return true;
-		}
+				return true;
+			}
 
-		return false;
+			return false;
+		}
 	}
 
 	/// <summary>