Refuse to schedule a reconnect while an attempt is in flight

Network/SipClient.cs

@@ -76,8 +76,8 @@
     public event Action<SipClient>? CallAnswer;
     public event Action<SipClient>? CallEnded;
     public event Action<SipClient, string>? StatusMessage;
     public event Action<SipClient>? RemotePutOnHold;
     public event Action<SipClient>? RemoteTookOffHold;
     public event Action<SipClient, Exception>? ErrorOccurred;
     public event Action<SipClient>? RegistrationStatusChanged;
     public event Action<SipClient, TimeSpan>? CallDurationUpdated;
@@ -139,7 +139,7 @@
     public string Username => _sipUsername;
     public string Server => _sipServer;
 
     public SipClient(
         SIPTransport sipTransport,
         SipConfig sipSettings,
         int registrationExpirySeconds = DefaultRegistrationExpirySeconds,
@@ -1101,11 +1101,16 @@
     /// Extended retry: the agent is stopped (the client owns the retries) and attempts continue
     /// forever, the delay doubling from the initial delay up to the cap.
     /// </remarks>
-    private void ScheduleReconnection()
+    internal void ScheduleReconnection()
     {
         lock (_lockObject)
         {
-            if (_isShutdown || _reconnectPending || _registrationState == RegistrationState.HardFailure)
+            // Checked under the lock: the health check reads the state without it, so by the time
+            // it gets here an attempt it did not know about may already be in flight. Scheduling
+            // then would start an attempt no failure asked for and, in extended mode, stop the
+            // in-flight agent before it can report its result.
+            if (_isShutdown || _reconnectPending
+                || _registrationState is RegistrationState.HardFailure or RegistrationState.Registering)
                 return;
 
             long delay;

tests/SipBotLib.Tests/RegistrationTemporaryFailureTests.cs

@@ -179,6 +179,34 @@ public async Task Health_check_does_not_interrupt_an_attempt_in_flight(bool exte
     }
 }
 
+/// <summary>
+/// The health check reads the registration state without the lock. If it read TemporaryFailure
+/// just before a scheduled attempt started, its ScheduleReconnection() call arrives while that
+/// attempt is in flight. Seen on the CI runner as more reconnect attempts than failures. The
+/// request must be refused, and the in-flight attempt must still report its own failure.
+/// </summary>
+public class RegistrationScheduleRaceTests
+{
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public async Task A_late_schedule_request_leaves_the_attempt_in_flight_alone(bool extended)
+    {
+        using var h = new RegistrationTestHarness(extended: extended);
+        h.Registrar.Script = Reply.Drop;
+        h.Client.StartRegistration();
+        Assert.Equal(RegistrationState.Registering, h.Client.RegistrationState);
+
+        h.Client.ScheduleReconnection();
+        await Task.Delay(600);
+
+        Assert.Equal(0, h.Metric("reconnection_attempts"));
+        Assert.Equal(1, h.Registrar.Count);
+        Assert.True(await h.WaitForState(RegistrationState.TemporaryFailure, TimeSpan.FromSeconds(5)),
+            $"the in-flight attempt never reported its failure; state is {h.Client.RegistrationState}");
+    }
+}
+
 /// <summary>
 /// After a success the extended backoff starts over: the first retry of the next outage comes
 /// after the initial delay again. Waits for a real refresh (25 s), so it is its own class.

tests/SipBotLib.Tests/RegistrationTestHarness.cs

@@ -49,6 +49,8 @@ public RegistrationTestHarness(
 
     public bool SawAlreadyRunning => Errors.Any(e => e.Message.Contains("already running"));
 
+    public long Metric(string name) => Client.Metrics.TryGetValue(name, out var value) ? value : 0;
+
     public static async Task<bool> WaitUntil(Func<bool> condition, TimeSpan timeout)
     {
         var deadline = DateTime.UtcNow + timeout;

tests/SipBotLib.Tests/TestSetup.cs

@@ -0,0 +1,19 @@
+using System.Runtime.CompilerServices;
+
+namespace SipBotLib.Tests;
+
+internal static class TestSetup
+{
+    /// <summary>
+    /// SIPSorcery blocks thread-pool threads (each registration attempt waits synchronously for
+    /// its response). With many test classes running in parallel on a small CI runner, the pool
+    /// runs dry and grows by about one thread a second, so timers fire up to ~1 s late and the
+    /// registration timing tests fail. A higher minimum keeps timers on time.
+    /// </summary>
+    [ModuleInitializer]
+    internal static void RaiseThreadPoolMinimum()
+    {
+        ThreadPool.GetMinThreads(out int workers, out int io);
+        ThreadPool.SetMinThreads(Math.Max(workers, 64), Math.Max(io, 64));
+    }
+}