Skip to content

Commit

Permalink
Dispose the prior response on retry dotnet#28384
Browse files Browse the repository at this point in the history
  • Loading branch information
Tratcher committed Sep 3, 2021
1 parent d9ac5f4 commit b885517
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 3 deletions.
18 changes: 15 additions & 3 deletions src/HttpClientFactory/Polly/src/PolicyHttpMessageHandler.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
Expand Down Expand Up @@ -73,6 +73,7 @@ namespace Microsoft.Extensions.Http
/// </remarks>
public class PolicyHttpMessageHandler : DelegatingHandler
{
private const string PriorResponseKey = "PolicyHttpMessageHandler.PriorResponse";
private readonly IAsyncPolicy<HttpResponseMessage> _policy;
private readonly Func<HttpRequestMessage, IAsyncPolicy<HttpResponseMessage>> _policySelector;

Expand Down Expand Up @@ -147,7 +148,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
/// <param name="context">The <see cref="Context"/>.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/>.</param>
/// <returns>Returns a <see cref="Task{HttpResponseMessage}"/> that will yield a response when completed.</returns>
protected virtual Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage request, Context context, CancellationToken cancellationToken)
protected virtual async Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage request, Context context, CancellationToken cancellationToken)
{
if (request == null)
{
Expand All @@ -159,7 +160,18 @@ protected virtual Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage req
throw new ArgumentNullException(nameof(context));
}

return base.SendAsync(request, cancellationToken);
if (request.Properties.TryGetValue(PriorResponseKey, out var priorResult) && priorResult is IDisposable disposable)
{
// This is a retry, dispose the prior response to free up the connection.
request.Properties.Remove(PriorResponseKey);
disposable.Dispose();
}

var result = await base.SendAsync(request, cancellationToken);

request.Properties.Add(PriorResponseKey, result);

return result;
}

private IAsyncPolicy<HttpResponseMessage> SelectPolicy(HttpRequestMessage request)
Expand Down
75 changes: 75 additions & 0 deletions src/HttpClientFactory/Polly/test/PolicyHttpMessageHandlerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading.Tasks;
using Microsoft.Extensions.Internal;
using Polly;
using Polly.Extensions.Http;
using Polly.Timeout;
using Xunit;

Expand Down Expand Up @@ -98,6 +99,54 @@ public async Task SendAsync_DynamicPolicy_PolicyTriggers_CanReexecuteSendAsync()
Assert.Same(expectedRequest, policySelectorRequest);
}

[Fact]
public async Task SendAsync_StaticPolicy_PolicyTriggers_CanReexecuteSendAsync_FirstResponseDisposed()
{
// Arrange
var policy = HttpPolicyExtensions.HandleTransientHttpError()
.RetryAsync(retryCount: 1);

var callCount = 0;
var fakeContent = new FakeContent();
var firstResponse = new HttpResponseMessage()
{
StatusCode = System.Net.HttpStatusCode.InternalServerError,
Content = fakeContent,
};
var expected = new HttpResponseMessage();

var handler = new PolicyHttpMessageHandler(policy);
handler.InnerHandler = new TestHandler()
{
OnSendAsync = (req, ct) =>
{
if (callCount == 0)
{
callCount++;
return Task.FromResult(firstResponse);
}
else if (callCount == 1)
{
callCount++;
return Task.FromResult(expected);
}
else
{
throw new InvalidOperationException();
}
}
};
var invoke = new HttpMessageInvoker(handler);

// Act
var response = await invoke.SendAsync(new HttpRequestMessage(), CancellationToken.None);

// Assert
Assert.Equal(2, callCount);
Assert.Same(expected, response);
Assert.True(fakeContent.Disposed);
}

[Fact]
public async Task SendAsync_DynamicPolicy_PolicySelectorReturnsNull_ThrowsException()
{
Expand Down Expand Up @@ -333,5 +382,31 @@ protected override Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage re
return OnSendAsync(request, context, cancellationToken);
}
}

private class TestHandler : HttpMessageHandler
{
public Func<HttpRequestMessage, CancellationToken, Task<HttpResponseMessage>> OnSendAsync { get; set; }

protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
Assert.NotNull(OnSendAsync);
return OnSendAsync(request, cancellationToken);
}
}

private class FakeContent : StringContent
{
public FakeContent() : base("hello world")
{
}

public bool Disposed { get; set; }

protected override void Dispose(bool disposing)
{
Disposed = true;
base.Dispose(disposing);
}
}
}
}

0 comments on commit b885517

Please sign in to comment.