Update client IceProtocolConnection to schedule first heartbeat only …

src/IceRpc/Internal/IceDuplexConnectionDecorator.cs

@@ -17,15 +17,8 @@ internal class IceDuplexConnectionDecorator : IDuplexConnection
     private readonly TimeSpan _writeIdleTimeout;
     private readonly Timer _writeTimer;
 
-    public async Task<TransportConnectionInformation> ConnectAsync(CancellationToken cancellationToken)
-    {
-        TransportConnectionInformation connectionInformation = await _decoratee.ConnectAsync(cancellationToken)
-            .ConfigureAwait(false);
-
-        // Schedule or reschedule a heartbeat after a successful connection establishment.
-        RescheduleWriteTimer();
-        return connectionInformation;
-    }
+    public Task<TransportConnectionInformation> ConnectAsync(CancellationToken cancellationToken) =>
+        _decoratee.ConnectAsync(cancellationToken);
 
     public void Dispose()
     {
@@ -103,10 +96,14 @@ internal IceDuplexConnectionDecorator(
         _readIdleTimeout = readIdleTimeout; // can be infinite i.e. disabled
         _writeIdleTimeout = writeIdleTimeout;
         _writeTimer = new Timer(_ => sendHeartbeat());
-
-        // We can't schedule a keep alive right away because the connection is not connected yet.
+        // We can't schedule the initial heartbeat yet. The heartbeat is an ice protocol frame; we can send it only once
+        // the connection is connected at the ice protocol level.
     }
 
+    /// <summary>Schedules the initial heartbeat. Called by a client IceProtocolConnection after it receives the
+    /// initial ValidateConnection frame from the server.</summary>
+    internal void ScheduleHeartbeat() => RescheduleWriteTimer();
+
     /// <summary>Cancels the write timer.</summary>
     private void CancelWriteTimer() => _writeTimer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
 

src/IceRpc/Internal/IceProtocolConnection.cs

@@ -117,8 +117,8 @@ internal sealed class IceProtocolConnection : IProtocolConnection
                     // Send ValidateConnection frame.
                     await SendControlFrameAsync(EncodeValidateConnectionFrame, connectCts.Token).ConfigureAwait(false);
 
-                    // The SendControlFrameAsync is a "write" that schedules a keep-alive when the idle timeout is not
-                    // infinite.
+                    // The SendControlFrameAsync is a "write" that schedules a heartbeat when the idle timeout is not
+                    // infinite. So no need to call ScheduleHeartbeat.
                 }
                 else
                 {
@@ -140,6 +140,12 @@ internal sealed class IceProtocolConnection : IProtocolConnection
                         throw new InvalidDataException(
                             $"Expected '{nameof(IceFrameType.ValidateConnection)}' frame but received frame type '{validateConnectionFrame.FrameType}'.");
                     }
+
+                    // The client connection is now connected, so we schedule the first heartbeat.
+                    if (_duplexConnection is IceDuplexConnectionDecorator decorator)
+                    {
+                        decorator.ScheduleHeartbeat();
+                    }
                 }
             }
             catch (OperationCanceledException)

tests/IceRpc.Tests/IceProtocolConnectionTests.cs

@@ -549,6 +549,43 @@ public async Task Idle_timeout_mismatch_aborts_connection()
             Throws.InstanceOf<IceRpcException>().With.Property("IceRpcError").EqualTo(IceRpcError.ConnectionAborted));
     }
 
+    /// <summary>Verifies that a connection where the client does not write anything is not aborted by the server
+    /// connection idle monitor.</summary>
+    /// <remarks>This test also verifies that the client idle monitor does not abort the connection when the server
+    /// does not write anything; it's less interesting since the server always writes a ValidateConnection frame after
+    /// accepting the connection from the client.</remarks>
+    [Test]
+    public async Task Server_idle_monitor_does_not_abort_connection_when_client_does_not_write_anything()
+    {
+        var connectionOptions = new ConnectionOptions
+        {
+            IceIdleTimeout = TimeSpan.FromMilliseconds(100)
+        };
+
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddProtocolTest(
+                Protocol.Ice,
+                dispatcher: null,
+                connectionOptions,
+                connectionOptions)
+            .BuildServiceProvider(validateScopes: true);
+
+        ClientServerProtocolConnection sut = provider.GetRequiredService<ClientServerProtocolConnection>();
+        (Task clientShutdownRequested, Task serverShutdownRequested) = await sut.ConnectAsync();
+
+        // Act
+        await Task.Delay(TimeSpan.FromMilliseconds(400)); // plenty of time for the idle monitor to kick in.
+
+        // Assert
+        Assert.That(serverShutdownRequested.IsCompleted, Is.False);
+        Assert.That(clientShutdownRequested.IsCompleted, Is.False);
+
+        // Graceful shutdown.
+        Task clientShutdown = sut.Client.ShutdownAsync();
+        Task serverShutdown = sut.Server.ShutdownAsync();
+        Assert.That(async () => await Task.WhenAll(clientShutdown, serverShutdown), Throws.Nothing);
+    }
+
     /// <summary>Verifies that canceling an invocation while the request is being written does not interrupt the write.
     /// The connection remains active and subsequent request are not affected.</summary>
     [Test]