Creating a singleton ChannelFactory<T> and reusing for client connections

Question

In our SharePoint/ASP.NET environment we have a series of data retriever classes that all derive from a common interface. I was assigned the task of creating a data retriever that could communicate remotely with other SharePoint farms using WCF. The way I have it implemented at the moment is a singleton ChannelFactory<T> is created in a static constructor and then reused by each instance of the remote data retriever to create an individual proxy instance. I figured that this would work well because then the ChannelFactory only gets instantiated once in an app domain and its creation is guaranteed to be thread-safe. My code looks something like this:

public class RemoteDataRetriever : IDataRetriever
{
    protected static readonly ChannelFactory<IRemoteDataProvider>
        RequestChannelFactory;

    protected IRemoteDataProvider _channel;

    static RemoteDataRetriever()
    {
        WSHttpBinding binding = new WSHttpBinding(
            SecurityMode.TransportWithMessageCredential, true);

        binding.Security.Transport.ClientCredentialType =
            HttpClientCredentialType.None;

        binding.Security.Message.ClientCredentialType =
            MessageCredentialType.Windows;

        RequestChannelFactory = 
            new ChannelFactory<IRemoteDataProvider>(binding);
    }

    public RemoteDataRetriever(string endpointAddress)
    {
        _channel = RemoteDataRetriever.RequestChannelFactory.
            CreateChannel(new EndpointAddress(endpointAddress));
    }
}

My question is, is this a good design? I figured that once the ChannelFactory is created I don't need to worry about thread-safety because I'm merely using it to call CreateChannel() but am I mistaken? Is it changing state or otherwise doing something funky behind the scenes that could cause threading issues? Additionally, do I need to put some code in somewhere (static finalizer?) that manually disposes of the ChannelFactory or can I assume that whenever IIS gets rebooted it'll do all the cleanup work for me?

Related: ChannelFactory Reuse Strategies

Gans

Answer 1

From a "is this singleton design good" well, your implementation of the Singleton is fine. It's thread-safe, and the ChannelFactory<T> is thread-safe as well.

You also don't have to worry about resource cleanup. Assuming that the ChannelFactory<T> follows Microsoft's guidelines for implementing IDisposable, then you won't have a problem with a leak of some kind. When an application domain is torn down, a garbage collection will be created, and everything will be cleaned up at that point. The finalizer on ChannelFactory<T> will do the cleanup one would normally do in the call to Dispose.

However, from a "should I be caching the ChannelFactory<T>" point of view, it's hard to say, because you don't indicate what version of .NET you are on. However, the article you point to indicates that if you are using .NET 3.0 SP1 or above, you really don't need to do this, you can create your proxies (assuming they derive from ClientBase<T>) where you need it in the client code, and not through a factory pattern like this.

Gualterio

I found a das blonde article that seems to indicate the same thing - dasblonde.net/…. However, the words "generated for you" put me off on the whole ClientBase<T> approach a little bit. Still, I think you're on the mark with this. – Gans

While doing research for another WCF problem I discovered a good reason to favor ChannelFactory<T> over ClientBase<T> - #2253247 – Gans

@Repo Man: If you prefer, but re-wrapping exceptions is not a best practice unless you have good reason to do so. For errors of this nature in WCF, I don't believe it qualifies for re-wrapping of exceptions. – Gualterio

Answer 2

As long as your "singleton" is simply returning newly constructed channels, you don't need to worry about thread safety. You can always throw the volatile keyword on the static declaration if you like as well, to prevent any compiler optimizations that could undermine you, but I don't think it's necessary in this case.

Just try to ask yourself questions about long-term flexibility. For example, what if later on you decided to add some state to this CreateChannel method? Perhaps it'd do something like create up to 10 channels, then start re-using them after that. Could you easily modify your singleton to do this?

Your answer is probably yes, but what if you then scale vertically to multiple servers? A singleton might not be sufficient. You'd need some way to share state amongst the instances/servers (e.g., database, distributed cache); that might be hard to do in a static context, depending on how you decided to share the state.

Housefly

Answer 3

I this article, Daniel Vaughan advocates caching Channels in a Singleton manager object but never caching a ChannelFactory. We've been using this approach with no problems success so far...

The benefits of doing so are:

"When the channel enters a faulted state it is removed from the cache and recreated when it is next requested..."

• Security negotiation is only performed once.
• Avoids having to explicitly close a channel on each use.
• We can add additional initialization functionality.
• We can fail early if the proxy is unable to communicate with the server.

Hypotaxis

I know it's old.. but wouldn't this effectively cache the ChannelFactory as well? Since releasing the Factory closes it's channels. – Service