From c093fe85b3a0c77d58fbb5fc3a489c2408626659 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Tue, 21 May 2024 20:37:39 -0400 Subject: [PATCH 1/4] More fixes for .NET 9.0 support --- tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj | 3 ++- tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj b/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj index 6b4c684ae..39c563206 100644 --- a/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj +++ b/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj @@ -64,7 +64,8 @@ - + tools/ true diff --git a/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props b/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props index b93ffb2ef..11150aaef 100644 --- a/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props +++ b/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props @@ -16,7 +16,7 @@ the source build. --> $(MSBuildThisFileDirectory)obj/tools/ - $(MSBuildThisFileDirectory)../IceRpc.ProtocGen/bin/$(Configuration)/net8.0/ + $(MSBuildThisFileDirectory)../IceRpc.ProtocGen/bin/$(Configuration)/$(TargetFramework)/ From 81295fb629f5dcf38ee24c7913c84331d206918b Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Tue, 21 May 2024 20:54:12 -0400 Subject: [PATCH 2/4] More fixes --- tests/IceRpc.Compressor.Tests/CompressorInterceptorTests.cs | 2 +- tests/IceRpc.Compressor.Tests/CompressorMiddlewareTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/IceRpc.Compressor.Tests/CompressorInterceptorTests.cs b/tests/IceRpc.Compressor.Tests/CompressorInterceptorTests.cs index 08b6d632b..f1b26493a 100644 --- a/tests/IceRpc.Compressor.Tests/CompressorInterceptorTests.cs +++ b/tests/IceRpc.Compressor.Tests/CompressorInterceptorTests.cs @@ -43,7 +43,7 @@ public async Task Compress_request_payload( new BrotliStream(outStream, CompressionMode.Decompress) : new DeflateStream(outStream, CompressionMode.Decompress); var decompressedPayload = new byte[4096]; - await decompressedStream.ReadAsync(decompressedPayload); + await decompressedStream.ReadAtLeastAsync(decompressedPayload, _payload.Length); Assert.That(decompressedPayload, Is.EqualTo(_payload)); payloadWriter.Complete(); } diff --git a/tests/IceRpc.Compressor.Tests/CompressorMiddlewareTests.cs b/tests/IceRpc.Compressor.Tests/CompressorMiddlewareTests.cs index bd4717e93..5e302b422 100644 --- a/tests/IceRpc.Compressor.Tests/CompressorMiddlewareTests.cs +++ b/tests/IceRpc.Compressor.Tests/CompressorMiddlewareTests.cs @@ -45,7 +45,7 @@ public async Task Compress_response_payload( new BrotliStream(outStream, CompressionMode.Decompress) : new DeflateStream(outStream, CompressionMode.Decompress); byte[] decompressedPayload = new byte[4096]; - await decompressedStream.ReadAsync(decompressedPayload); + await decompressedStream.ReadAtLeastAsync(decompressedPayload, _payload.Length); Assert.That(decompressedPayload, Is.EqualTo(_payload)); payloadWriter.Complete(); } From 1c545cc7dbb26df3696e29c06b7b7ad2e2ddcd53 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 22 May 2024 10:39:51 -0400 Subject: [PATCH 3/4] Add new KeepAliveInterval option in QuicClientTransportOptions --- .../Internal/QuicMultiplexedListener.cs | 3 ++ .../QuicClientTransport.cs | 3 ++ .../QuicTransportOptions.cs | 24 ++++++++-- .../Transports/QuicIdleTimeoutTests.cs | 48 +++++++++++++++++-- .../IceRpc.Protobuf.Tools.csproj | 3 +- .../IceRpc.Protobuf.Tools.props | 2 +- 6 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/IceRpc.Transports.Quic/Internal/QuicMultiplexedListener.cs b/src/IceRpc.Transports.Quic/Internal/QuicMultiplexedListener.cs index 7f20f7493..c85ab8dda 100644 --- a/src/IceRpc.Transports.Quic/Internal/QuicMultiplexedListener.cs +++ b/src/IceRpc.Transports.Quic/Internal/QuicMultiplexedListener.cs @@ -61,6 +61,9 @@ internal QuicMultiplexedListener( DefaultCloseErrorCode = (int)MultiplexedConnectionCloseError.Aborted, DefaultStreamErrorCode = 0, IdleTimeout = quicTransportOptions.IdleTimeout, +#if NET9_0_OR_GREATER + KeepAliveInterval = Timeout.InfiniteTimeSpan, // the server doesn't send PING frames +#endif ServerAuthenticationOptions = authenticationOptions, MaxInboundBidirectionalStreams = options.MaxBidirectionalStreams, MaxInboundUnidirectionalStreams = options.MaxUnidirectionalStreams diff --git a/src/IceRpc.Transports.Quic/QuicClientTransport.cs b/src/IceRpc.Transports.Quic/QuicClientTransport.cs index 42addac9b..e29a6a075 100644 --- a/src/IceRpc.Transports.Quic/QuicClientTransport.cs +++ b/src/IceRpc.Transports.Quic/QuicClientTransport.cs @@ -65,6 +65,9 @@ public IMultiplexedConnection CreateConnection( DefaultCloseErrorCode = (int)MultiplexedConnectionCloseError.Aborted, DefaultStreamErrorCode = 0, IdleTimeout = _quicTransportOptions.IdleTimeout, +#if NET9_0_OR_GREATER + KeepAliveInterval = _quicTransportOptions.KeepAliveInterval, +#endif LocalEndPoint = _quicTransportOptions.LocalNetworkAddress, RemoteEndPoint = endpoint, MaxInboundBidirectionalStreams = options.MaxBidirectionalStreams, diff --git a/src/IceRpc.Transports.Quic/QuicTransportOptions.cs b/src/IceRpc.Transports.Quic/QuicTransportOptions.cs index 17f76e393..c4ce58be5 100644 --- a/src/IceRpc.Transports.Quic/QuicTransportOptions.cs +++ b/src/IceRpc.Transports.Quic/QuicTransportOptions.cs @@ -9,9 +9,11 @@ public record class QuicTransportOptions { /// Gets or sets the idle timeout. This timeout is used to monitor the transport connection health. If no /// data is received within the idle timeout period, the transport connection is aborted. - /// The idle timeout. Defaults to meaning that the underlying QUIC - /// implementation default will be used. - public TimeSpan IdleTimeout { get; set; } = TimeSpan.Zero; + /// The idle timeout. Defaults to 30 seconds. means "use the default value + /// provided by the underlying implementation". + /// The idle timeout is negotiated by QUIC during connection establishment. The actual idle timeout value + /// for a connection can be lower than the value you've set here. + public TimeSpan IdleTimeout { get; set; } = TimeSpan.FromSeconds(30); } /// The options class for configuring . @@ -22,6 +24,22 @@ public sealed record class QuicClientTransportOptions : QuicTransportOptions /// establishment. /// The address and port to bind to. Defaults to . public IPEndPoint? LocalNetworkAddress { get; set; } + + /// Gets or sets the interval at which the client sends QUIC PING frames to the server to keep the + /// connection alive. + /// The keep-alive interval. Defaults to 15 seconds. means: use the default + /// value provided by the underlying implementation. means: never send + /// PING frames. + /// Unlike the idle timeout, the keep-alive interval is not negotiated during connection establishment. We + /// recommend setting this interval at half the negotiated idle timeout value to keep a healthy connection alive + /// forever - really until it's closed due to inactivity at the RPC level via the + /// . + /// This option is a stop-gap: ideally the .NET QUIC implementation would provide a way to automatically send PING + /// frames to keep the connection alive based on the negotiated idle timeout. See + /// . + /// This option is only implemented for .NET 9.0. With .NET 8.0, neither the client nor the server sends + /// PING frames to keep the connection alive. + public TimeSpan KeepAliveInterval { get; set; } = TimeSpan.FromSeconds(15); } /// The options class for configuring . diff --git a/tests/IceRpc.Quic.Tests/Transports/QuicIdleTimeoutTests.cs b/tests/IceRpc.Quic.Tests/Transports/QuicIdleTimeoutTests.cs index 331828511..c37e1279d 100644 --- a/tests/IceRpc.Quic.Tests/Transports/QuicIdleTimeoutTests.cs +++ b/tests/IceRpc.Quic.Tests/Transports/QuicIdleTimeoutTests.cs @@ -26,7 +26,7 @@ public void FixtureSetUp() /// timeout. /// The behavior shown by this test is not desirable for IceRPC: we would prefer QUIC or IceRPC's QUIC /// wrapper to keep this connection alive when there is an outstanding request. - /// See https://github.com/icerpc/icerpc-csharp/issues/3353. + /// See . [Test] public async Task Quic_connection_idle_after_idle_timeout([Values]bool configureServer) { @@ -60,8 +60,6 @@ public async Task Quic_connection_idle_after_idle_timeout([Values]bool configure // Act / Assert var startTime = TimeSpan.FromMilliseconds(Environment.TickCount64); - Assert.That(readResult.Buffer.ToArray(), Is.EqualTo(data)); - Assert.That( async () => await sut.Local.Input.ReadAsync().AsTask(), Throws.InstanceOf().With.Property("IceRpcError").EqualTo(IceRpcError.ConnectionIdle)); @@ -69,5 +67,49 @@ public async Task Quic_connection_idle_after_idle_timeout([Values]bool configure Assert.That( TimeSpan.FromMilliseconds(Environment.TickCount64) - startTime, Is.GreaterThan(TimeSpan.FromMilliseconds(490))); + + Assert.That(readResult.Buffer.ToArray(), Is.EqualTo(data)); + } + + /// Verifies the QUIC connection is kept alive by the keep alive interval. + [Test] + public async Task Quic_connection_kept_alive_by_keep_alive_interval() + { + // Arrange + var services = new ServiceCollection().AddQuicTest(); + + services.AddOptions("server").Configure( + options => options.IdleTimeout = TimeSpan.FromMilliseconds(500)); + + services.AddOptions("client").Configure( + options => options.KeepAliveInterval = TimeSpan.FromMilliseconds(250)); + + await using ServiceProvider provider = services.BuildServiceProvider(validateScopes: true); + + var clientServerConnection = provider.GetRequiredService(); + await clientServerConnection.AcceptAndConnectAsync(); + using var sut = await clientServerConnection.CreateAndAcceptStreamAsync(bidirectional: true); + + // Simulate a request + var data = new byte[] { 0x1, 0x2, 0x3 }; + await sut.Local.Output.WriteAsync(data); + ReadResult readResult = await sut.Remote.Input.ReadAsync(); + + // Act/Assert + + // Without the keep-alive interval, the idle timer aborts the connection after 500ms. + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2)); + +#if NET9_0_OR_GREATER + Assert.That( + async () => await sut.Local.Input.ReadAsync(cts.Token), + Throws.InstanceOf()); +#else + Assert.That( + async () => await sut.Local.Input.ReadAsync().AsTask(), + Throws.InstanceOf().With.Property("IceRpcError").EqualTo(IceRpcError.ConnectionIdle)); +#endif + + Assert.That(readResult.Buffer.ToArray(), Is.EqualTo(data)); } } diff --git a/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj b/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj index 39c563206..6b4c684ae 100644 --- a/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj +++ b/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.csproj @@ -64,8 +64,7 @@ - + tools/ true diff --git a/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props b/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props index 11150aaef..b93ffb2ef 100644 --- a/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props +++ b/tools/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props @@ -16,7 +16,7 @@ the source build. --> $(MSBuildThisFileDirectory)obj/tools/ - $(MSBuildThisFileDirectory)../IceRpc.ProtocGen/bin/$(Configuration)/$(TargetFramework)/ + $(MSBuildThisFileDirectory)../IceRpc.ProtocGen/bin/$(Configuration)/net8.0/ From dc5b80820d654916bfb7a0a1b24a337c6e3593ff Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 22 May 2024 14:35:59 -0400 Subject: [PATCH 4/4] Update test --- .../Transports/QuicIdleTimeoutTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/IceRpc.Quic.Tests/Transports/QuicIdleTimeoutTests.cs b/tests/IceRpc.Quic.Tests/Transports/QuicIdleTimeoutTests.cs index c37e1279d..17472b207 100644 --- a/tests/IceRpc.Quic.Tests/Transports/QuicIdleTimeoutTests.cs +++ b/tests/IceRpc.Quic.Tests/Transports/QuicIdleTimeoutTests.cs @@ -93,23 +93,23 @@ public async Task Quic_connection_kept_alive_by_keep_alive_interval() // Simulate a request var data = new byte[] { 0x1, 0x2, 0x3 }; await sut.Local.Output.WriteAsync(data); - ReadResult readResult = await sut.Remote.Input.ReadAsync(); + ReadResult incomingRequest = await sut.Remote.Input.ReadAsync(); // Act/Assert - // Without the keep-alive interval, the idle timer aborts the connection after 500ms. - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2)); + // Without the keep-alive interval PINGs, the idle timer would abort the connection. + await Task.Delay(TimeSpan.FromSeconds(2)); #if NET9_0_OR_GREATER - Assert.That( - async () => await sut.Local.Input.ReadAsync(cts.Token), - Throws.InstanceOf()); + // Verify the connection and stream still work by sending and receiving a "response". + await sut.Remote.Output.WriteAsync(data); + ReadResult incomingResponse = await sut.Local.Input.ReadAsync(); + Assert.That(incomingResponse.Buffer.ToArray(), Is.EqualTo(data)); #else Assert.That( async () => await sut.Local.Input.ReadAsync().AsTask(), Throws.InstanceOf().With.Property("IceRpcError").EqualTo(IceRpcError.ConnectionIdle)); #endif - - Assert.That(readResult.Buffer.ToArray(), Is.EqualTo(data)); + Assert.That(incomingRequest.Buffer.ToArray(), Is.EqualTo(data)); } }