WaitAndRetryAsync causes app hang when retrying a "503 - Service unavailable" response

[marcpaulv]

Summary:
When the Application Pool of an IIS hosted website stops for whatever reason (in my case i stopped it manually), IIS returns a 503 - Service unavailable status code and adds a Connection: close response header.

When retrying such a request with the WaitAndRetryAsync policy, the first 2 retries are returning the same status (503) but the third retry is causing the application to stop responding.

Steps to reproduce

1. Copy the below code into a .Net framework 4.7.2 console application
2. Import the following Nuget packages

  <PackageReference Include="Microsoft.Extensions.DependencyInjection">
      <Version>2.1.1</Version>
    </PackageReference>
    <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions">
      <Version>2.1.1</Version>
    </PackageReference>
    <PackageReference Include="Microsoft.Extensions.Http">
      <Version>2.1.1</Version>
    </PackageReference>
    <PackageReference Include="Microsoft.Extensions.Http.Polly">
      <Version>2.1.1</Version>
    </PackageReference>
    <PackageReference Include="Polly">
      <Version>7.0.3</Version>
    </PackageReference>

2. Replace URLPlaceholder with a URL pointing to an IIS hosted API hosted on the network (not on localhost)
3. After receiving a couple of 200 OK requests, manually Stop the Application pool of the IIS hosted API
4. After receiving a couple of 503 Service Unavailable responses, manually Start the Application pool of the IIS hosted API

Expected behavior:
The application should start receiving 200 OK responses.

Actual behaviour:
The application hangs after the first 2 retries of the first 503 Service unavailable response.

Note that if the WaitAndRetryAsync policy addition is commented out, the application behaves as expected (without the retries of course).

Code to reproduce the problem:

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Http;
using Polly;
using Polly.Timeout;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Net.Sockets;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace ConsoleApp2
{
    class Program
    {
        async static Task Main(string[] args)
        {
            IServiceCollection services = new ServiceCollection();
            services.AddHttpClient("test")
              // comment out the below policy for a correct behavior
               .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]{
               TimeSpan.FromSeconds(1),
               TimeSpan.FromSeconds(2),
               TimeSpan.FromSeconds(4),
               }));
  
              // removing the logging handlers as a work around for https://github.com/aspnet/Extensions/issues/563
            ServiceDescriptor loggingHandler = services.FirstOrDefault(e => e.ServiceType == typeof(IHttpMessageHandlerBuilderFilter));
            if (loggingHandler != null)
            {
                services.Remove(loggingHandler);
            }

            IHttpClientFactory factory = services.BuildServiceProvider().GetService<IHttpClientFactory>();

            HttpClient client = factory.CreateClient("test");

            while (true)
            {
                HttpResponseMessage response = null;
                try
                {
                    response = await client.GetAsync("URLplaceholder");
                }
                catch (Exception e)
                {
                    // logging
                }

                Thread.Sleep(5000);
            }
        }
    }
}

Observations

If you retry the request only once, the behavior is as expected.
The problem seems to appear when you retry more than once with the same HttpClient instance.

[marcpaulv]

@reisenberger any thoughts ?

[gethari]

@marcpaulv We are facing this issue today. Did you get any update on this ?

[marcpaulv]

hi @gethari, no update on this from this repo, unfortunately.

I did log another issue on https://github.com/dotnet/extensions/issues/3385. I didn't get any replies there either, but I found a workaround by talking to miself :))

[Dragonsangel]

As a cross-referencing since the issue was reported in the other repository as well, I created a self-contained reproducing solution for this occurrence under https://github.com/Dragonsangel/PollyConnectionsRemainOpen
In the reproducing solution there are no deadlock or hanging, but the core of the problem is shown. Being that the connections are kept open upon receiving a 5xx Error from a query and that a new connection is created for each retry.

[AKlaus]

As dotnet/aspnetcore/issues/28384 suggests, the issue might have been resolved in .NET 6 RC2.

Meanwhile for older versions, the advised solution is to explicitly dispose the HttpResponseMessage on retries (suggestion). Though, it didn't help me.

HttpPolicyExtensions.HandleTransientHttpError().WaitAndRetryAsync(
	new[] { TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(2), TimeSpan.FromSeconds(4) },
	(response, _, _, _) => response?.Result.Dispose()
)

[github-actions]

This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made.

[Dragonsangel]

We are still facing this issue and should not be closed.

[martintmk]

As a cross-referencing since the issue was reported in the other repository as well, I created a self-contained reproducing solution for this occurrence under https://github.com/Dragonsangel/PollyConnectionsRemainOpen In the reproducing solution there are no deadlock or hanging, but the core of the problem is shown. Being that the connections are kept open upon receiving a 5xx Error from a query and that a new connection is created for each retry.

Hey @Dragonsangel , this problem is fixed in Polly V8 as it automatically disposes discarded results.

See:

Polly/src/Polly.Core/Retry/RetryResilienceStrategy.cs

Line 85 in dd511b2

await DisposeHelper.TryDisposeSafeAsync(resultValue, context.IsSynchronous).ConfigureAwait(context.ContinueOnCapturedContext);

I am not sure if this will be backported to V7.