views:

89

answers:

1

Hello,

I've been spending about an hour searching for a concensus on something I'm trying to accomplish, but have yet to find anything conclusive in a particular direction.

My situation is as follows:

  • I have a multi-threaded application (.NET web service)
  • I have classes that use objects that take non-negligible time to load, so I would like to maintain them as static class members
  • The code that constructs these objects intermittently has a low chance of failure

I was previously using an approach that constructs these objects in a static constructor. The problem with this was that, as mentioned above, the constructor would occasionally fail, and once a .NET static constructor fails, the whole class is hosed until the process is restarted. There are no second chances with that approach.

The most intuitive-seeming approach after this was to use double-checked locking. There are a lot of pages around that talk about the evils of double-checked locking and say to use a static constructor, which I was already doing, but that doesn't seem to be an option for me, as the static constructor has the potential to fail and bring down the whole class.

The implementation (simplified, of course) I'm thinking of using is the following. All class and member names are purely demonstrative and not what I'm actually using. Is this approach going to be problematic? Can anyone suggest a better approach?

public class LazyMembers
{
    private static volatile XmlDocument s_doc;
    private static volatile XmlNamespaceManager s_nsmgr;
    private static readonly object s_lock = new object();

    private static void EnsureStaticMembers()
    {
        if (s_doc == null || s_nsmgr == null)
        {
            lock (s_lock)
            {
                if (s_doc == null || s_nsmgr == null)
                {
                    // The following method might fail
                    // with an exception, but if it succeeds,
                    // s_doc and s_nsmgr will be initialized
                    s_doc = LoadDoc(out s_nsmgr);
                }
            }
        }
    }

    public XmlNamespaceManager NamespaceManager
    {
        get
        {
            EnsureStaticMembers();
            return s_nsmgr;
        }
    }

    public XmlDocument GetDocClone()
    {
        EnsureStaticMembers();
        return (XmlDocument)s_doc.Clone();
    }
}
+1  A: 

If you use .NET 4.0 you can refer to Lazy<T> and LazyThreadSafetyMode (it depends on whether you want few instances of T to be created or not in multithreaded environment. In your case you need to refer to Lazy<T>(Func<T> func, LazyThreadSafetyMode) constructor - here(MSDN)

Otherwise (if you use 3.5 or lower) you can CAS techinque to create single instance without locking.

Something like this:

get {
    if( _instance == null) {
        var singleton = new Singleton();
        if(Interlocked.CompareExchange(ref _instance, singleton, null) != null) {
            if (singleton is IDisposable) singleton.Dispose();
        }
    }
    return _instance;
}

However, here you can only achieve LazyThreadSafetyMode.Publications behaviour - only one instance will be visible to other threads but few can be created.

Also, there should not be any problems with double check for null in your code - it's safe in .NET world (at least on x86 machines and associated memory model). There were some problems in Java world before 2004, AFAIK.

Andrei Taptunov
Thank you for your input. The lock() approach seems simpler and better suited since I'm working with two variables to initialize, but I will definitely consider using Interlocked.CompareExchange() for future problems. I'm not using .NET 4.0, so Lazy<T> isn't an option at the moment.
Jimmy