From cc5d1818959779b12e264777c0c6e98983d32df5 Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 10 Jun 2022 14:30:13 -0500 Subject: [PATCH] #703 drop ConnectionLeaseTimeout --- Test/Flurl.Test/Http/RealHttpTests.cs | 28 ----------- Test/Flurl.Test/Http/SettingsTests.cs | 14 ------ .../Configuration/FlurlHttpSettings.cs | 10 ---- src/Flurl.Http/FlurlClient.cs | 48 ++----------------- 4 files changed, 3 insertions(+), 97 deletions(-) diff --git a/Test/Flurl.Test/Http/RealHttpTests.cs b/Test/Flurl.Test/Http/RealHttpTests.cs index 6f68ad60..91d45b45 100644 --- a/Test/Flurl.Test/Http/RealHttpTests.cs +++ b/Test/Flurl.Test/Http/RealHttpTests.cs @@ -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(); - var tasks = new List(); - - // 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(() => 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(); diff --git a/Test/Flurl.Test/Http/SettingsTests.cs b/Test/Flurl.Test/Http/SettingsTests.cs index c45da28c..e0d593ae 100644 --- a/Test/Flurl.Test/Http/SettingsTests.cs +++ b/Test/Flurl.Test/Http/SettingsTests.cs @@ -278,20 +278,6 @@ public void can_provide_custom_client_factory() { Assert.IsInstanceOf(cli.HttpClient); Assert.IsInstanceOf(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] diff --git a/src/Flurl.Http/Configuration/FlurlHttpSettings.cs b/src/Flurl.Http/Configuration/FlurlHttpSettings.cs index 8693ae8a..df2c9213 100644 --- a/src/Flurl.Http/Configuration/FlurlHttpSettings.cs +++ b/src/Flurl.Http/Configuration/FlurlHttpSettings.cs @@ -174,16 +174,6 @@ internal void Set(T value, [CallerMemberName]string propName = null) { /// public class ClientFlurlHttpSettings : FlurlHttpSettings { - /// - /// 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. - /// - public TimeSpan? ConnectionLeaseTimeout { - get => Get(); - set => Set(value); - } - /// /// 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, diff --git a/src/Flurl.Http/FlurlClient.cs b/src/Flurl.Http/FlurlClient.cs index 982497dc..10e1b856 100644 --- a/src/Flurl.Http/FlurlClient.cs +++ b/src/Flurl.Http/FlurlClient.cs @@ -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; @@ -64,7 +62,7 @@ public class FlurlClient : IFlurlClient /// /// The base URL associated with this client. public FlurlClient(string baseUrl = null) { - _httpClient = new Lazy(CreateHttpClient); + _httpClient = new Lazy(() => Settings.HttpClientFactory.CreateHttpClient(HttpMessageHandler)); _httpMessageHandler = new Lazy(() => Settings.HttpClientFactory.CreateMessageHandler()); BaseUrl = baseUrl; } @@ -86,7 +84,7 @@ public FlurlClient(HttpClient httpClient) { /// public ClientFlurlHttpSettings Settings { - get => _settings ?? (_settings = new ClientFlurlHttpSettings()); + get => _settings ??= new ClientFlurlHttpSettings(); set => _settings = value; } @@ -94,47 +92,7 @@ public ClientFlurlHttpSettings Settings { public INameValueList Headers { get; } = new NameValueList(false); // header names are case-insensitive https://stackoverflow.com/a/5259004/62600 /// - 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(CreateHttpClient); - _httpMessageHandler = new Lazy(() => 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; /// public HttpMessageHandler HttpMessageHandler => HttpTest.Current?.HttpMessageHandler ?? _httpMessageHandler?.Value;