How to do a Multithread-Safe, Atomic Object Creation and Swap

In WCF setting up a communication channel is a somewhat expensive operation, so it is recommended to set one up and share it throughout your application's lifetime. One big caveat is that if the channel ever faults then that channel has to be aborted and replaced, these zombie channels cannot be resurrected.

What I'm trying to do is write a custom proxy that takes care of keeping a healthy channel created and providing access to its operations. The sticky part comes when the channel faults and I need to swap it for a new, good channel in a threadsafe manner.

Here is the code I have so far, I'm still a bit of an apprentice when it comes to the black art of thread safe programming. Is this code overkill, underkill, just plain wrong? Can the same thing be accomplished in a quicker, simpler, or more correct manner?

public class WcfDeviceActivationService : IDeviceActivationService {
    ChannelFactory<IDeviceActivationService> _factory = new ChannelFactory<IDeviceActivationService>();
    IDeviceActivationService _channel = null;

    ReaderWriterLockSlim _swaplock = new ReaderWriterLockSlim();

    public WcfDeviceActivationService() {
        _channel = _factory.CreateChannel();
        ((IClientChannel)_channel).Open();
    }

    public Guid ActivateDevice(string activationCode, Guid userId) {
        return Call(c => c.ActivateDevice(activationCode, userId) );
    }

    private RT Call<RT>(Func<IDeviceActivationService, RT> chanfunc) {
        try {
            using(_swaplock.UseReadLock())
                return chanfunc(_channel);
        } catch(Exception) {
            // Get a re开发者_StackOverflow社区ference to the channel before attempting the exclusive lock
            var chan = _channel;

            // Take an exclusive lock to block callers while we check the channel
            // (just to prevent excessive failures under heavy call load)
            using (_swaplock.UseWriteLock()) {
                // Let's see if we're still working with the original channel and if it's faulted
                if (Object.ReferenceEquals(chan, _channel) && ((IClientChannel)chan).State == CommunicationState.Faulted) {
                    // It faulted, so lets create a new channel to replace the bad one

                    // If the channel creation throws, the next attempt to use the failed channel
                    //  will take this path again and attempt to create a fresh channel.
                    // We want the creation exception to propagate so that the caller
                    //  knows that their call failed because the channel couldn't be created.
                    var newchan = _factory.CreateChannel();
                    ((IClientChannel)newchan).Open();

                    // Exchange the new channel for the old one
                    // (assigning reference types is atomic, but Exchange also does a memory barrier for us)
                    Interlocked.Exchange(ref _channel, newchan);

                    // Clean up the old channel
                    ((IClientChannel)chan).Abort();
                }
            }

            // Propagate exception to the caller
            throw;
        }
    }
}

UPDATE

Thinking through simplifying the code thanks to Fredrik and mlaw, I've refactored to this, still not sure if it's totally correct or not.

---

You could do an Interlocked.CompareExchange with the dead channel passed in as comparand and the new channel passed in as value.

If the exchange succeeds, great.
If the exchange fails, then someone already updated the dead channel.

The only issue is that you may, occasionally, create an extra channel that you just have to throw away after you find that CompareExchange fails. Is this acceptable in your situation?

The benefit is that CompareExchange is probably non-blocking (not 100% sure as I'm a Java guy, not .Net. I'm basing this assumption on Java's AtomicReference.compareAndSet).