Redisign HTTP/2 KeepAlive PING tests (#56736)

Completely redesign tests for HTTP/2 KeepAlive PING, so they:
- Work well with RTT pings introduced in Implement dynamic HTTP/2 window scaling #54755
- Run sequentially, reducing the chance of failing because of timing issues caused by parallel workloads
- Are better organized: multiple test cases for different scenarios, instead of one theory with complex branches on parameters

Instead of reading / reacting to frames inline, there is a separate Task for processing incoming frames, responding to PING immediately and pushing other frames to a Channel<Frame>.

Fixes #41929

Author: Anton Firszov

src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs

@@ -24,7 +24,6 @@ public class Http2LoopbackConnection : GenericLoopbackConnection
         private Stream _connectionStream;
         private TaskCompletionSource<bool> _ignoredSettingsAckPromise;
         private bool _ignoreWindowUpdates;
-        private TaskCompletionSource<PingFrame> _expectPingFrame;
         private bool _transparentPingResponse;
         private readonly TimeSpan _timeout;
         private int _lastStreamId;
@@ -201,7 +200,7 @@ public async Task<Frame> ReadFrameAsync(CancellationToken cancellationToken)
                 return await ReadFrameAsync(cancellationToken).ConfigureAwait(false);
             }
 
-            if (header.Type == FrameType.Ping && (_expectPingFrame != null || _transparentPingResponse))
+            if (header.Type == FrameType.Ping && _transparentPingResponse)
             {
                 PingFrame pingFrame = PingFrame.ReadFrom(header, data);
 
@@ -237,13 +236,7 @@ public async Task<Frame> ReadFrameAsync(CancellationToken cancellationToken)
 
         private async Task<bool> TryProcessExpectedPingFrameAsync(PingFrame pingFrame)
         {
-            if (_expectPingFrame != null)
-            {
-                _expectPingFrame.SetResult(pingFrame);
-                _expectPingFrame = null;
-                return true;
-            }
-            else if (_transparentPingResponse && !pingFrame.AckFlag)
+            if (_transparentPingResponse && !pingFrame.AckFlag)
             {
                 try
                 {
@@ -293,22 +286,6 @@ public void IgnoreWindowUpdates()
             _ignoreWindowUpdates = true;
         }
 
-        // Set up loopback server to expect a PING frame among other frames.
-        // Once PING frame is read in ReadFrameAsync, the returned task is completed.
-        // The returned task is canceled in ReadPingAsync if no PING frame has been read so far.
-        // Does not work when Http2Options.EnableTransparentPingResponse == true
-        public Task<PingFrame> ExpectPingFrameAsync()
-        {
-            if (_transparentPingResponse)
-            {
-                throw new InvalidOperationException(
-                    $"{nameof(Http2LoopbackConnection)}.{nameof(ExpectPingFrameAsync)} can not be used when transparent PING response is enabled.");
-            }
-
-            _expectPingFrame ??= new TaskCompletionSource<PingFrame>();
-            return _expectPingFrame.Task;
-        }
-
         public async Task ReadRstStreamAsync(int streamId)
         {
             Frame frame = await ReadFrameAsync(_timeout);
@@ -772,9 +749,6 @@ public async Task PingPong()
 
         public async Task<PingFrame> ReadPingAsync(TimeSpan timeout)
         {
-            _expectPingFrame?.TrySetCanceled();
-            _expectPingFrame = null;
-
             Frame frame = await ReadFrameAsync(timeout).ConfigureAwait(false);
             Assert.NotNull(frame);
             Assert.Equal(FrameType.Ping, frame.Type);

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs

@@ -1206,7 +1206,7 @@ private Task SendSettingsAckAsync() =>
         private Task SendPingAsync(long pingContent, bool isAck = false) =>
             PerformWriteAsync(FrameHeader.Size + FrameHeader.PingLength, (thisRef: this, pingContent, isAck), static (state, writeBuffer) =>
             {
-                if (NetEventSource.Log.IsEnabled()) state.thisRef.Trace("Started writing.");
+                if (NetEventSource.Log.IsEnabled()) state.thisRef.Trace($"Started writing. {nameof(pingContent)}={state.pingContent}");
 
                 Debug.Assert(sizeof(long) == FrameHeader.PingLength);
 

src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs

@@ -1783,149 +1783,6 @@ public async Task Http2_SendOverConnectionWindowSizeWithoutExplicitFlush_ClientS
             }
         }
 
-        public static IEnumerable<object[]> KeepAliveTestDataSource()
-        {
-            yield return new object[] { Timeout.InfiniteTimeSpan, HttpKeepAlivePingPolicy.Always, false };
-            yield return new object[] { TimeSpan.FromSeconds(1), HttpKeepAlivePingPolicy.WithActiveRequests, false };
-            yield return new object[] { TimeSpan.FromSeconds(1), HttpKeepAlivePingPolicy.Always, false };
-            yield return new object[] { TimeSpan.FromSeconds(1), HttpKeepAlivePingPolicy.WithActiveRequests, true };
-        }
-
-        [OuterLoop("Significant delay.")]
-        [MemberData(nameof(KeepAliveTestDataSource))]
-        [ConditionalTheory(nameof(SupportsAlpn))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/41929")]
-        public void Http2_PingKeepAlive(TimeSpan keepAlivePingDelay, HttpKeepAlivePingPolicy keepAlivePingPolicy, bool expectRequestFail)
-        {
-            RemoteExecutor.Invoke(RunTest, keepAlivePingDelay.Ticks.ToString(), keepAlivePingPolicy.ToString(), expectRequestFail.ToString()).Dispose();
-
-            static async Task RunTest(string keepAlivePingDelayString, string keepAlivePingPolicyString, string expectRequestFailString)
-            {
-                // We should refactor this test so it can react to RTT PINGs.
-                // For now, avoid interference by disabling them:
-                AppContext.SetSwitch("System.Net.SocketsHttpHandler.Http2FlowControl.DisableDynamicWindowSizing", true);
-
-                bool expectRequestFail = bool.Parse(expectRequestFailString);
-                TimeSpan keepAlivePingDelay = TimeSpan.FromTicks(long.Parse(keepAlivePingDelayString));
-                HttpKeepAlivePingPolicy keepAlivePingPolicy = Enum.Parse<HttpKeepAlivePingPolicy>(keepAlivePingPolicyString);
-
-                TimeSpan pingTimeout = TimeSpan.FromSeconds(5);
-                // Simulate failure by delaying the pong, otherwise send it immediately.
-                TimeSpan pongDelay = expectRequestFail ? pingTimeout * 2 : TimeSpan.Zero;
-                // Pings are send only if KeepAlivePingDelay is not infinite.
-                bool expectStreamPing = keepAlivePingDelay != Timeout.InfiniteTimeSpan;
-                // Pings (regardless ongoing communication) are send only if sending is on and policy is set to always.
-                bool expectPingWithoutStream = expectStreamPing && keepAlivePingPolicy == HttpKeepAlivePingPolicy.Always;
-
-                TaskCompletionSource serverFinished = new TaskCompletionSource();
-
-                await Http2LoopbackServer.CreateClientAndServerAsync(
-                    async uri =>
-                    {
-                        SocketsHttpHandler handler = new SocketsHttpHandler()
-                        {
-                            KeepAlivePingTimeout = pingTimeout,
-                            KeepAlivePingPolicy = keepAlivePingPolicy,
-                            KeepAlivePingDelay = keepAlivePingDelay
-                        };
-                        handler.SslOptions.RemoteCertificateValidationCallback = delegate { return true; };
-
-                        using HttpClient client = new HttpClient(handler);
-                        client.DefaultRequestVersion = HttpVersion.Version20;
-
-                        // Warmup request to create connection.
-                        await client.GetStringAsync(uri);
-                        // Request under the test scope.
-                        if (expectRequestFail)
-                        {
-                            await Assert.ThrowsAsync<HttpRequestException>(() => client.GetStringAsync(uri));
-                            // As stream is closed we don't want to continue with sending data.
-                            return;
-                        }
-                        else
-                        {
-                            await client.GetStringAsync(uri);
-                        }
-
-                        // Let connection live until server finishes.
-                        try
-                        {
-                            await serverFinished.Task.WaitAsync(pingTimeout * 3);
-                        }
-                        catch (TimeoutException) { }
-                    },
-                    async server =>
-                    {
-                        using Http2LoopbackConnection connection = await server.EstablishConnectionAsync();
-
-                        Task<PingFrame> receivePingTask = expectStreamPing ? connection.ExpectPingFrameAsync() : null;
-
-                        // Warmup the connection.
-                        int streamId1 = await connection.ReadRequestHeaderAsync();
-                        await connection.SendDefaultResponseAsync(streamId1);
-
-                        // Request under the test scope.
-                        int streamId2 = await connection.ReadRequestHeaderAsync();
-
-                        // Test ping with active stream.
-                        if (!expectStreamPing)
-                        {
-                            await Assert.ThrowsAsync<OperationCanceledException>(() => connection.ReadPingAsync(pingTimeout));
-                        }
-                        else
-                        {
-                            PingFrame ping;
-                            if (receivePingTask != null && receivePingTask.IsCompleted)
-                            {
-                                ping = await receivePingTask;
-                            }
-                            else
-                            {
-                                ping = await connection.ReadPingAsync(pingTimeout);
-                            }
-                            if (pongDelay > TimeSpan.Zero)
-                            {
-                                await Task.Delay(pongDelay);
-                            }
-
-                            await connection.SendPingAckAsync(ping.Data);
-                        }
-
-                        // Send response and close the stream.
-                        if (expectRequestFail)
-                        {
-                            await Assert.ThrowsAsync<IOException>(() => connection.SendDefaultResponseAsync(streamId2));
-                            // As stream is closed we don't want to continue with sending data.
-                            return;
-                        }
-                        await connection.SendDefaultResponseAsync(streamId2);
-                        // Test ping with no active stream.
-                        if (expectPingWithoutStream)
-                        {
-                            PingFrame ping = await connection.ReadPingAsync(pingTimeout);
-                            await connection.SendPingAckAsync(ping.Data);
-                        }
-                        else
-                        {
-                            // If the pings were recently coming, just give the connection time to clear up streams
-                            // and still accept one stray ping.
-                            if (expectStreamPing)
-                            {
-                                try
-                                {
-                                    await connection.ReadPingAsync(pingTimeout);
-                                }
-                                catch (OperationCanceledException) { } // if it failed once, it will fail again
-                            }
-                            await Assert.ThrowsAsync<OperationCanceledException>(() => connection.ReadPingAsync(pingTimeout));
-                        }
-                        serverFinished.SetResult();
-                        await connection.WaitForClientDisconnectAsync(true);
-                    },
-                    new Http2Options() { EnableTransparentPingResponse = false });
-            }
-        }
-
         [OuterLoop("Uses Task.Delay")]
         [ConditionalFact(nameof(SupportsAlpn))]
         public async Task Http2_MaxConcurrentStreams_LimitEnforced()

src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.Http2FlowControl.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Diagnostics;
 using System.Linq;
 using System.Net.Test.Common;
 using System.Threading;
@@ -236,7 +235,6 @@ private static async Task<int> TestClientWindowScalingAsync(
 
                 int nextRemainingBytes = remainingBytes - bytesToSend;
                 bool endStream = nextRemainingBytes == 0;
-
                 await writeSemaphore.WaitAsync();
                 Interlocked.Add(ref credit, -bytesToSend);
                 await connection.SendResponseDataAsync(streamId, responseData, endStream);