Skip to content

Commit

Permalink
#703 drop ConnectionLeaseTimeout
Browse files Browse the repository at this point in the history
  • Loading branch information
tmenier committed Jun 10, 2022
1 parent 1b7aec6 commit cc5d181
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 97 deletions.
28 changes: 0 additions & 28 deletions Test/Flurl.Test/Http/RealHttpTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,34 +233,6 @@ public void can_set_timeout_and_cancellation_token() {
Assert.IsFalse(cts.Token.IsCancellationRequested);
}

[Test, Ignore("failing on AppVeyor, holding up bugfix release")]
public async Task connection_lease_timeout_doesnt_disrupt_calls() {
// testing this quickly is tricky. HttpClient will be replaced by a new instance after 1 timeout and disposed
// after another, so the timeout period (typically minutes in real-world scenarios) needs to be long enough
// that we don't dispose before the response from google is received. 1 second seems to work.
var cli = new FlurlClient("http://www.google.com");
cli.Settings.ConnectionLeaseTimeout = TimeSpan.FromMilliseconds(1000);

var httpClients = new List<HttpClient>();
var tasks = new List<Task>();

// ping google for about 2.5 seconds
for (var i = 0; i < 25; i++) {
if (!httpClients.Contains(cli.HttpClient))
httpClients.Add(cli.HttpClient);
tasks.Add(cli.Request().HeadAsync());
await Task.Delay(100);
}
await Task.WhenAll(tasks); // failed HTTP status, etc, would throw here and fail the test.

Assert.AreEqual(3, httpClients.Count);

// only the first one should be disposed, which isn't particularly simple to check
Assert.ThrowsAsync<ObjectDisposedException>(() => httpClients[0].GetAsync("http://www.google.com"));
await httpClients[1].GetAsync("http://www.google.com");
await httpClients[2].GetAsync("http://www.google.com");
}

[Test]
public async Task test_settings_override_client_settings() {
var cli1 = new FlurlClient();
Expand Down
14 changes: 0 additions & 14 deletions Test/Flurl.Test/Http/SettingsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -278,20 +278,6 @@ public void can_provide_custom_client_factory() {
Assert.IsInstanceOf<SomeCustomHttpClient>(cli.HttpClient);
Assert.IsInstanceOf<SomeCustomMessageHandler>(cli.HttpMessageHandler);
}

[Test]
public async Task connection_lease_timeout_creates_new_HttpClient() {
var cli = new FlurlClient("http://api.com");
cli.Settings.ConnectionLeaseTimeout = TimeSpan.FromMilliseconds(50);
var hc = cli.HttpClient;

await Task.Delay(25);
Assert.That(hc == cli.HttpClient);

// exceed the timeout
await Task.Delay(25);
Assert.That(hc != cli.HttpClient);
}
}

[TestFixture, Parallelizable]
Expand Down
10 changes: 0 additions & 10 deletions src/Flurl.Http/Configuration/FlurlHttpSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,6 @@ internal void Set<T>(T value, [CallerMemberName]string propName = null) {
/// </summary>
public class ClientFlurlHttpSettings : FlurlHttpSettings
{
/// <summary>
/// Specifies the time to keep the underlying HTTP/TCP connection open. When expired, a Connection: close header
/// is sent with the next request, which should force a new connection and DSN lookup to occur on the next call.
/// Default is null, effectively disabling the behavior.
/// </summary>
public TimeSpan? ConnectionLeaseTimeout {
get => Get<TimeSpan?>();
set => Set(value);
}

/// <summary>
/// Gets or sets a factory used to create the HttpClient and HttpMessageHandler used for HTTP calls.
/// Whenever possible, custom factory implementations should inherit from DefaultHttpClientFactory,
Expand Down
48 changes: 3 additions & 45 deletions src/Flurl.Http/FlurlClient.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Net.Http;
using Flurl.Http.Configuration;
using Flurl.Http.Testing;
Expand Down Expand Up @@ -64,7 +62,7 @@ public class FlurlClient : IFlurlClient
/// </summary>
/// <param name="baseUrl">The base URL associated with this client.</param>
public FlurlClient(string baseUrl = null) {
_httpClient = new Lazy<HttpClient>(CreateHttpClient);
_httpClient = new Lazy<HttpClient>(() => Settings.HttpClientFactory.CreateHttpClient(HttpMessageHandler));
_httpMessageHandler = new Lazy<HttpMessageHandler>(() => Settings.HttpClientFactory.CreateMessageHandler());
BaseUrl = baseUrl;
}
Expand All @@ -86,55 +84,15 @@ public FlurlClient(HttpClient httpClient) {

/// <inheritdoc />
public ClientFlurlHttpSettings Settings {
get => _settings ?? (_settings = new ClientFlurlHttpSettings());
get => _settings ??= new ClientFlurlHttpSettings();
set => _settings = value;
}

/// <inheritdoc />
public INameValueList<string> Headers { get; } = new NameValueList<string>(false); // header names are case-insensitive https://stackoverflow.com/a/5259004/62600

/// <inheritdoc />
public HttpClient HttpClient => HttpTest.Current?.HttpClient ?? _injectedClient ?? GetHttpClient();

private DateTime? _clientCreatedAt;
private HttpClient _zombieClient;
private readonly object _connectionLeaseLock = new object();

private HttpClient GetHttpClient() {
if (ConnectionLeaseExpired()) {
lock (_connectionLeaseLock) {
if (ConnectionLeaseExpired()) {
// when the connection lease expires, force a new HttpClient to be created, but don't
// dispose the old one just yet - it might have pending requests. Instead, it becomes
// a zombie and is disposed on the _next_ lease timeout, which should be safe.
_zombieClient?.Dispose();
_zombieClient = _httpClient.Value;
_httpClient = new Lazy<HttpClient>(CreateHttpClient);
_httpMessageHandler = new Lazy<HttpMessageHandler>(() => Settings.HttpClientFactory.CreateMessageHandler());
_clientCreatedAt = DateTime.UtcNow;
}
}
}
return _httpClient.Value;
}

private HttpClient CreateHttpClient() {
var cli = Settings.HttpClientFactory.CreateHttpClient(HttpMessageHandler);
_clientCreatedAt = DateTime.UtcNow;
return cli;
}

private bool ConnectionLeaseExpired() {
// for thread safety, capture these to temp variables
var createdAt = _clientCreatedAt;
var timeout = Settings.ConnectionLeaseTimeout;

return
_httpClient.IsValueCreated &&
createdAt.HasValue &&
timeout.HasValue &&
DateTime.UtcNow - createdAt.Value > timeout.Value;
}
public HttpClient HttpClient => HttpTest.Current?.HttpClient ?? _injectedClient ?? _httpClient.Value;

/// <inheritdoc />
public HttpMessageHandler HttpMessageHandler => HttpTest.Current?.HttpMessageHandler ?? _httpMessageHandler?.Value;
Expand Down

0 comments on commit cc5d181

Please sign in to comment.