Singleton WCF Client doesn't respect DNS changes
edavedian opened this issue · 32 comments
@mconnew @Lxiamail There is a long thread about a similar topic here https://github.com/dotnet/corefx/issues/11224 which discusses the issue with singleton HttpClient. In .NET Core 2.1, this has been solved by the introduction of HttpClientFactory and in the thread it is also mentioned that the use of SocketsHttpHandler as a possible resolution. However, the creation of the HttpClient and HttpClientHandler in https://github.com/dotnet/wcf/blob/master/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/HttpChannelFactory.cs is completely encapsulated and there is no opportunity for the application to either provide an instance of HttpClient created by HttpClientFactory or an instance of SocketsHttpHandler. What is the best way to address this issue with the WCF client?
there is no opportunity for the application to either provide an instance of HttpClient created by HttpClientFactory or an instance of SocketsHttpHandler.
Luckily this statement is incorrect!! But you can be forgiven for not knowing about how to do this as it's not actually exposed via an obvious or documentable API and I only introduced it for .Net Core 2.1 (WCF package versions 4.5+ I think). I implemented the ability to provide a Func<HttpClientHandler, HttpMessageHandler> to enable modifying or replacing the HttpMessageHandler. You provide a method which takes an HttpClientHandler and returns an HttpMessageHandler. The reason for the mismatch in input and output types is to provide maximum flexibility and maximum information.
Internally we instantiate HttpClientHandler and pass that in to the method. This allows you to have full access to every property we set on the HttpClientHandler without using a potentially fragile cast (prevents us from forgetting about users of this extensibility where we could switch to SocketsHttpHandler in a future release and break people casting).
The return type is HttpMessageHandler to allow developers to return any compatible type they want. HttpClient only demands an HttpMessageHandler so this gives you the freedom of creating your own layered handler or completely replacing the instance with a brand new instance of SocketsHttpHandler. This is where the strongly typed HttpClientHandler is important as you will need access to all the properties such as credentials to make your replacement instance work.
You could provide an instance of HttpMessageHandler which replaces an inner handler which does the actual work on a periodic basis. Or one which does a DNS lookup once the TTL has expired and if the set of IP addresses changes, replaces an inner handler with a new one. Lots of options. You can see an example implementation which simply allows modifying the request before it's sent or the response before it's returned here. You need a behavior which installs the Func which will be used to modify/replace the used handler.
Or if you want a simpler but slightly dirtier solution, in .Net Core the HttpClient for WCF is only scoped to the instance of ChannelFactory (unlike on the .Net Framework where the connection pool is shared across identically configured instances, but I provided a way to override that on .Net Framework a few years ago too!!). The dirty solution is to create a new ChannelFactory and throw away the old one on a periodic basis. This can be pretty rough on the GC if you do it too often and your contracts are large as a ChannelFactory can be hundreds of MB if your contract is very large.
@mconnew Thank you very much for the detailed explanation. I implemented Func<HttpClientHandler, HttpMessageHandler> as you had suggested and just out of curiosity, I examined the instance of HttpClientHandler that was provided in the debugger. I can see a private member _socketsHttpHandler which is an instance of SocketsHttpHandler and I believe that is due to https://docs.microsoft.com/en-us/dotnet/core/whats-new/dotnet-core-2-1#sockets-improvements.
So I assume that HttpClientHandler that is provided is an instance of SocketsHttpHandler but it doesn't look like any type of casting from the provided HttpClientHandler to SocketsHttpHandler works. Do you see a way to set the PooledConnectionLifetime of the SocketsHttpHandler that is a member of the provided HttpClientHandler in Func<HttpClientHandler, HttpMessageHandler>? I would rather just update the instance that was provided.
There are multiple reasons we use HttpClientHandler and not SocketsHttpHandler. The first is that if we added a dependency to SocketsHttpHandler, our code would no longer run on .Net Core 2.0. Our package is listed as a netstandard2.0 package and not a netcoreapp package so this would be a problem for us. Especially as SocketsHttpHandler isn't supported for UAP/UWP which would cause us problems. Another reason is because there are app settings that can be set to cause HttpClientHandler to wrap the older handler implementation from .Net Core 2.0 and earlier. We've had a few people need to do that due to some bugs in the newer SocketsHttpHandler implementation. If we had instantiated an instance of SocketsHttpHandler instead, then we would be fixed to that implementation and anyone wanting to switch back would need to go to all the effort of supplying a Func and mapping the settings across themselves to switch back. SocketsHttpHandler is wrapped inside of HttpClientHandler so no cast is possible. You have two options. 1. Use reflection to get the underlying instance of SocketsHttpHandler and modify it. You can just return the original HttpClientHandler from your method. 2. Instantiate your own SocketsHttpHandler and copy the appropriate values from HttpClientHandler across. You can see the set of properties we modify here, it's not many. The exception to that is if you are using HTTPS in which case we might set HttpClientHandler.ServerCertificateCustomValidationCallback and specify a client certificate if you are using one. The amount of code to translate across in most use cases would only be 4 or 5 lines.
@mconnew You have been tremendously helpful. I really appreciate your help. My preference was to use option 1 because as you suspected for some of our testing environments we need to use self signed certificates. I therefore have this
Type type = typeof(HttpClientHandler);
SocketsHttpHandler socketsHandler = (SocketsHttpHandler)type.GetField("_socketsHttpHandler", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(httpClientHandler);
socketsHandler.PooledConnectionLifetime = TimeSpan.FromSeconds(60);
httpClientHandler.ServerCertificateCustomValidationCallback = (message, cert, chain, errors) => { return true; };
return httpClientHandler;
But unfortunately that results in this exception
at System.ServiceModel.ClientBase`1.get_Channel()
at System.ServiceModel.ClientBase`1.CreateChannel()
at System.ServiceModel.ChannelFactory`1.CreateChannel()
at System.ServiceModel.ChannelFactory`1.CreateChannel(EndpointAddress address, Uri via)
at System.ServiceModel.ChannelFactory.EnsureOpened()
at System.ServiceModel.Channels.CommunicationObject.ThrowIfDisposed()
System.ServiceModel.CommunicationObjectFaultedException: The communication object, System.ServiceModel.ChannelFactory`1[COLPEntitlementService.EntitlementService], cannot be used for communication because it is in the Faulted state.
But if I just do and not use SSL due to self signed certificates
var handler = new SocketsHttpHandler()
{
PooledConnectionLifetime = TimeSpan.FromSeconds(60)
};
return handler;
Then it works fine. Any idea why updating the existing instance causes the exception?
Something in your delegate is throwing an exception which is causing the channel to be faulted. You could attach a debugger and you should see the exception thrown. Or you could create a test app and create an instance of HttpClientHandler and then execute your code and see if it throws.
Also, WCF can wire up the certificate validation for you. Use code similar to this:
factory.Credentials.ServiceCertificate.SslCertificateAuthentication = new X509ServiceCertificateAuthentication();
factory.Credentials.ServiceCertificate.SslCertificateAuthentication.CertificateValidationMode = X509CertificateValidationMode.None;@mconnew Thank you very much. Really appreciate it. Just out of curiosity, are there any plans for WCF to integrate with HttpClientFactory since that has resolved a lot of issues related to using singleton HttpClient?
We can't because it's not a part of netstandard and is an ASP.NET Core feature. It would definitely be possible to create a Func which returns a handler which internally delegates the request using HttpClientFactory, but that's not something we can ship in-box.
@mconnew Are you referring to the current supported delegate Func<HttpClientHandler, HttpMessageHandler> for what you mentioned?
Yes. Nothing stopping you from creating a class which derives from HttpMessageHandler and in the SendAsync method uses HttpClientFactory to get an instance to make the actual call. You would need to save any relevant settings from the passed in HttpClientHandler such as credentials and certificate validation callback etc in your message handler to make sure you configured HttpClientHandler correctly. But it's totally doable.
Thank you very much. Really appreciate your help.
@mconnew I just had one last clarification on using HttpClientFactory in HttpMessageHandler chain of WCF client. If we make the HTTP call in HttpMessageHandler's SendAsync, once that is completed, won't the message handler chain continue and then send the message again as it is not aware that the message has already been sent and received?
@edavedian, it doesn't automatically chain the handlers. When you return a replacement handler, it's up to you to call the original handler if you want them to chain. If you derive from DelegatingHandler, then you provide the chained handler to DelegatingHandler and when you call base.SendAsync it will call that chained handler. But it's up to your code to do that. If you simply dervive from HttpMessageHandler, it's up to you to chain the original handler if that's what you want.
@mconnew thank you very much and sorry for the long delay. Do you not see any issues/side effects with having the settings SendTimeout, ReceiveTimeout and other settings in the Binding when using a new instance of HttpClient from HttpClientFactory in HttpMessageHandler?
We use the SendTimeout/ReceiveTimeout to create a cancellation token. I would suggest you disable the timeouts on HttpClient itself (I think we set it to 0 but it might be -1 for infinite or TimeSpan.MaxValue, I can't remember which off hand) otherwise it will cancel all requests at 30 seconds. It also creates a LinkedCancellationToken for each request which has other overhead. WCF creates a CancellationToken for each request and passes that to the SendAsync method so you don't need to worry about propagating any timeout values other than disabling the HttpClient defaults.
@mconnew I just got around doing what you suggested above which is to send the request in the new instance of HttpClientHandler created when Func<HttpClientHandler, HttpMessageHandler> is called by WCF. The new HttpClientHandler is simple as below
public class ColpWcfHttpClientHandler : HttpClientHandler
{
private IHttpClientFactory _httpClientFactory;
public ColpWcfHttpClientHandler(IHttpClientFactory httpClientFactory)
{
_httpClientFactory = httpClientFactory;
}
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
HttpClient httpClient = _httpClientFactory.CreateClient(HttpNamedClients.Colp);
return httpClient.SendAsync(request, cancellationToken);
}
}
But this causes the below exception:
System.InvalidOperationException:` The request message was already sent. Cannot send the same request message multiple times.
at System.Runtime.AsyncResult.End[TAsyncResult](IAsyncResult result)
at System.ServiceModel.Channels.ServiceChannel.SendAsyncResult.End(SendAsyncResult result)
at System.ServiceModel.Channels.ServiceChannel.EndCall(String action, Object[] outs, IAsyncResult result)
at System.ServiceModel.Channels.ServiceChannelProxy.TaskCreator.<>c__DisplayClass1_0.b__0(IAsyncResult asyncResult)
--- End of stack trace from previous location where exception was thrown ---
@edavedian HttpChannelFactory owns its HttpClient instances. Func<HttpClientHandler, HttpMessageHandler> is providing the HttpMessageHandler that is passed into HttpClient's constructor.
By the time your ColpWcfHttpClientHandler.SendAsync is called, we're already inside a call to HttpClient.SendAsync. HttpClient.SendAsync marks the request as sent here: https://github.com/dotnet/corefx/blob/0ace0eb7068169a48df9f4748b1adc15ce9cb845/src/System.Net.Http/src/System/Net/Http/HttpClient.cs#L422
And then ColpWcfHttpClientHandler.SendAsync will get called a few lines down here: https://github.com/dotnet/corefx/blob/0ace0eb7068169a48df9f4748b1adc15ce9cb845/src/System.Net.Http/src/System/Net/Http/HttpClient.cs#L455
So, this means we can't use our own HttpClient to try to send the message, because another HttpClient is already in the middle of trying to send it. Instead of IHttpClientFactory, if you use IHttpMessageHandlerFactory and use it to build a HttpMessageHandler to pass to Func<HttpClientHandler, HttpMessageHandler> then it should work.
@mconnew To follow up on something you said earlier in this issue:
The dirty solution is to create a new ChannelFactory and throw away the old one on a periodic basis. This can be pretty rough on the GC if you do it too often and your contracts are large as a ChannelFactory can be hundreds of MB if your contract is very large.
Does this mean we should re-use the same instance of ClientBase, and should not recreate it for every request? What is using up so much memory in ChannelFactory?
@bdrupieski, your earlier comment about using IHttpMessageHandlerFactory is correct. Inside ColpWcfHttpClientHandler, a new MessageHandler should be created and then use that.
Now to your question, it's the ContractDescription which can get large. It's normally rebuilt with each ChannelFactory from your service contract interface. We've just added another set of API's from the full .NET Framework which should be able to help with this. ClientBase and ChannelFactory both have constructors which take a ServiceEndpoint class. This will allow you to reuse the ContractDescription for multiple instances of ChannelFactory and reduce the allocations needed when creating multiple instances. This will be released as part of .NET Core 3.0. If you need it sooner, the ChannelFactory constructor is there in the implementation, just not the contract so you could potentially use reflection to use it. The ClientBase constructor is completely absent though.
@mconnew @bdrupieski thank you very much for your suggestions. Using IHttpMessageHandlerFactory did work. But in addition to using this mechanism to try to address the problem for which this issue was opened, I was trying to address another issue with socket saturation which I am having with WCF running on Linux as part of .NET core 2.2 compilation (and this doesn't occur when deployed on Windows). I will open a new issue for that. Appreciate your help again.
There is an article explaining how all this works in practice
https://habr.com/en/company/true_engineering/blog/465421/
@mconnew no, just applied the solution of the author on our microservices and it works quite fine. Then thought will be like a good conclusion for this thread of discussions.
There is an article explaining how all this works in practice
https://habr.com/en/company/true_engineering/blog/465421/
This link is broken :(
The translated version of the blog post had images that were not translated. Here is a 100% English version of the same blog post:
https://medium.com/trueengineering/realization-of-the-connections-pool-with-wcf-for-net-core-with-usage-of-httpclientfactory-c2cb2676423e
@mconnew i have an exception in my code but not sure what is wrong with this code
private TService GetServiceInstance<TService, TChannel>(string serviceUrl, string username, string password)
where TChannel : class
where TService : System.ServiceModel.ClientBase<TChannel>
{
var binding = new System.ServiceModel.BasicHttpBinding
{
MaxBufferSize = int.MaxValue,
ReaderQuotas = System.Xml.XmlDictionaryReaderQuotas.Max,
MaxReceivedMessageSize = int.MaxValue,
AllowCookies = true,
ReceiveTimeout = this.ReceiveTimeout,
SendTimeout = this.SendTimeout
};
binding.Security.Transport.ClientCredentialType = System.ServiceModel.HttpClientCredentialType.Basic;
binding.Security.Mode = System.ServiceModel.BasicHttpSecurityMode.TransportCredentialOnly;
var endpoint = new System.ServiceModel.EndpointAddress(serviceUrl);
object[] args = new object[] { binding, endpoint };
var client = (TService)Activator.CreateInstance(typeof(TService), args);
client.ClientCredentials.UserName.UserName = username;
client.ClientCredentials.UserName.Password = password;
IHttpMessageHandlerFactory httpMessageHandler = _serviceProvider.GetService(typeof(IHttpMessageHandlerFactory)) as IHttpMessageHandlerFactory;
client.Endpoint.EndpointBehaviors.Add(new CustomEndpointBehavior(httpMessageHandler));
return client;
}
public async Task<Data> GetDataFromMyClient()
{
try
{
ServicePointManager.Expect100Continue = false;
MyClient client = GetServiceInstance<MyClient, MyReceiver>(this.ServiceUrl, this.username, this.password);
var response = await client.MyMethodAsync();
return response;
}
catch (Exception ex)
{
_logger.LogError(ex.ToString());
}
return null;
} public class CustomEndpointBehavior : IEndpointBehavior
{
private readonly Func<HttpMessageHandler> _httpHandler;
public CustomEndpointBehavior(IHttpMessageHandlerFactory factory) => _httpHandler = () => factory.CreateHandler("ClientName");
public void AddBindingParameters(ServiceEndpoint endpoint, BindingParameterCollection bindingParameters) => bindingParameters.Add(new Func<HttpClientHandler, HttpMessageHandler>(handler => _httpHandler()));
public void ApplyClientBehavior(ServiceEndpoint endpoint, ClientRuntime clientRuntime) { }
public void ApplyDispatchBehavior(ServiceEndpoint endpoint, EndpointDispatcher endpointDispatcher) { }
public void Validate(ServiceEndpoint endpoint) { }
}services.AddHttpClient("ClientName");
Exception details:
System.ServiceModel.Security.MessageSecurityException: The HTTP request is unauthorized with client authentication scheme 'Basic'. The authentication header received from the server was 'Basic realm="JDBCRealm"'.
at System.Runtime.AsyncResult.End[TAsyncResult](IAsyncResult result)
at System.ServiceModel.Channels.ServiceChannel.SendAsyncResult.End(SendAsyncResult result)
at System.ServiceModel.Channels.ServiceChannel.EndCall(String action, Object[] outs, IAsyncResult result)
at System.ServiceModel.Channels.ServiceChannelProxy.TaskCreator.<>c__DisplayClass1_0.<CreateGenericTask>b__0(IAsyncResult asyncResult)
NOTE: I dont get any exception when I dont use CustomEndpointBehavior
It's not working because you are outright replacing the HttpClientHandler that WCF gives you. This line:
binding.Security.Transport.ClientCredentialType = System.ServiceModel.HttpClientCredentialType.Basic;Combined with these lines:
client.ClientCredentials.UserName.UserName = username;
client.ClientCredentials.UserName.Password = password;Results in HttpClientHandler.Credentials being populated with an instance of CredentialCache which holds a NetworkCredential for that username and password configured to for use with the "Basic" auth scheme. You then replace the HttpClientHandler with a MessageHandler that comes from IHttpMessageHandlerFactory.CreateHandler() which doesn't have any credentials populated. As the return type from the Func is a MessageHandler (in order to allow you to replace it with anything you like), WCF can't populate any credentials afterwards. Any needed credentials or configuration such as service certificate validation is all configured on the HttpClientHandler that's passed in and you are expected to copy over to your own handler anything that you need.
On a completely unrelated note, a small comment on your code. You can simplify your call to GetService by using a generic parameter and it's probably best to use GetRequiredService in your case as that one throws if it can't provide it. GetService will return null if it can't provide it which is probably not what you want to do here. My suggestion is to use this code instead:
_serviceProvider.GetRequiredService<IHttpMessageHandlerFactory>();As it's generic, the return type will be IHttpMessageHandlerFactory and you don't need to do a cast.
@mconnew thank you very much for reply and advices.
How to set the passed HttpClientHandler Credentials to the newly created handler?
You use the IHttpClientBuilder returned from AddHttpClient like to set your own properties on the HttpClientHandler like this:
services.AddHttpClient("ClientName")
.ConfigurePrimaryHttpMessageHandler(() =>
{
return new HttpClientHandler()
{
UseDefaultCredentials = false,
Credentials = new NetworkCredential(username, password),
};
});Docs are here.
This is the simple form which is probably sufficient for your use case. WCF uses a CredentialCache instead of NetworkCredential as it can constrain the credentials to not be used with less secure authentication mechanisms. For example if you expect to use Negotiate (Windows auth), then you can prevent a server requesting Basic auth which actually sends the username and password base64 encoded and tells the server what your plain text password is. You might not want that, but as you are using basic auth anyway, there's no way to downgrade the security any further so a simple NetworkCredential is sufficient. I'm saying this more for anyone that follows and reads this rather than for your scenario. There are other properties which WCF sets which aren't set by doing this. For example any certificate validation for the server cert (you should be using HTTPS because of basic auth). We also configure compression support, whether cookies are persisted, and we turn on pre authentication to skip an extra unauthenticated call.
I'm curious why you are needing to use IHttpClientFactory? Are you planning on using policies? Are are you trying to avoid a stale DNS issue?
In our use case we have different wcf credentials per user, this means I cant add the credentials on ConfigurePrimaryHttpMessageHandler.
The main reason for me to use IHttpClientFactory is reuse the HttpClient and prevent socket exhaustion, in our project we have about 30 different wcf endpoints and used by about 210k users very much.
As you are using Basic auth, it might be easier for you to just add the authentication header yourself. This way you can use a single ChannelFactory instance for all users. The basic auth protocol is simply add an Authentication header with the value Basic [base64encode(username:password)]. So for example, if your username is bob and your password is ShhItsASecret, you would base64 encode the string bob:ShhItsASecret and set the Authentication header to that. So code might look like this:
var credsBytes = System.Text.Encoding.UTF8.GetBytes("bob:ShhItsASecret");
var encodedCreds = System.Convert.ToBase64String(credsBytes);
var authHeaderValue = "Basic " + encodedCreds;
Task resultTask;
using (OperationContextScope scope = new OperationContextScope((IContextChannel)client))
{
HttpRequestMessageProperty requestMessageProperty;
if (!OperationContext.Current.OutgoingMessageProperties.ContainsKey(HttpRequestMessageProperty.Name))
{
requestMessageProperty = new HttpRequestMessageProperty();
OperationContext.Current.OutgoingMessageProperties[HttpRequestMessageProperty.Name] = requestMessageProperty;
}
else
{
requestMessageProperty = (HttpRequestMessageProperty)OperationContext.Current.OutgoingMessageProperties[HttpRequestMessageProperty.Name];
}
requestMessageProperty.Headers["Authentication"] = authHeaderValue;
resultTask = serviceProxy.DoSomethingAsync();
}
// await needs to happen outside of using block otherwise you get an exception due to OperationContextScope not supporting async properly yet
await resultTask;If you use this approach, you can actually just use a single ChannelFactory instance for all endpoints. ChannelFactory has an overload on CreateChannel which takes an EndpointAddress. So you can do this:
ClientBase<TService> myClient; // This represents a singleton instance of your client class
var channel = myClient.ChannelFactory.CreateChannel(new EndpointAddress("http://myserveraddress/myapp.svc");
((IClientChannel)channel).Open();
// Use channel like in previous code example
((IClientChannel)channel).Close();Make sure you open and close your channel. The channel factory keeps track of the channels and if you don't close it, holds a reference and you will keep growing your memory usage. Opening it prevents another problem which can happen if multiple threads try to use the same channel before it's implicitly opened. Using a few helper methods, you should be able to make this method really simple to use.