C# Singleton Thread Safety

Question

I read Jon Skeet's definitive post on how to implement C# singleton and am following below pattern. Note that I do not have an empty ctor. My Ctor may do some work such as creating and populating an array of strings (or it might create some objects and assign them to private variables etc.):

public class MyClass
    {                
        /// <summary>
        /// Get singleton instance of this class.
        /// </summary>
        public static readonly MyClass Instance = new MyClass();


        /// <summary>
        /// a collection of strings.
        /// </summary>
        private string[] strings;        

        private MyClass()
        {            
            this.strings = new string[]
            {
                "a",
                "b",
                "c",
                "d",
                "e" 
            };
        }

        public void MyMethod()
        {
           // tries to use this.strings.
           // can a null ref exception happen here when running multithreaded code?
        }
}

is above threadsafe? I ask because I have similar code running on asp.net appserver and get null ref exception in the logs (not sure if the null ref is related to above - I think not - and the call stack in log is not helpful).

[Yes, the initialization is thread-safe](http://stackoverflow.com/a/12159883/21475). It's guaranteed to happen just once, and only one instance of your class will be instantiated. — Cameron, Oct 15 '14 at 17:23
Yes it's thread safe. An extra info - if in case, you want singleton "per thread" you could mark the static field with ThreadStatic attribute. — gp., Oct 15 '14 at 17:28

score 1 · Answer 1 · edited May 23 '17 at 11:56

I honestly don't see a reason why it shouldn't be thread safe. Especially considering that Jon's fourth thread safe version is essentially the same.

The only problem that I see is that you don't have a static constructor. (this could cause problems, see this) If you add the static constructor (even if it's empty), you'll have what Jon Skeet calls thread-safe.

public class MyClass
{
    public static readonly MyClass Instance = new MyClass();

    // Explicit static constructor to tell C# compiler
    // not to mark type as beforefieldinit
    static MyClass() { }
}

score 1 · Answer 2 · answered Oct 15 '14 at 17:43

According to the mentioned Jon Skeet article, the addition of a static constructor will cause this implementation to be thread safe:

The laziness of type initializers is only guaranteed by .NET when the type isn't marked with a special flag called beforefieldinit. Unfortunately, the C# compiler (as provided in the .NET 1.1 runtime, at least) marks all types which don't have a static constructor (i.e. a block which looks like a constructor but is marked static) as beforefieldinit

(see http://csharpindepth.com/articles/general/singleton.aspx#cctor)

As you have it now, it is not thread safe. If you do something like this it becomes thread safe:

public class MyClass
{
    /// <summary>
    /// Get singleton instance of this class
    /// </summary>
    public static readonly MyClass Instance = new MyClass();

    static MyClass()
    {
        //causes the compiler to not mark this as beforefieldinit, giving this thread safety
        //for accessing the singleton.
    }

    //.. the rest of your stuff..
}

i thought beforefieldinit is only controlling the laziness of type initializer. it shouldn't impact the thread safety? — morpheus, Oct 15 '14 at 17:58
@Morpheus: that is correct. The original example is thread-safe, but may not be initialized lazily. Personally, if lazy init was important to me, I'd implement the singleton using the Lazy class (I think that Skeet's article predates that class, unless he's updated it since originally writing it). — Peter Duniho, Oct 15 '14 at 18:36
the lazy initialization is not important for me. so keeping that aside, is there any problem with the code posted in the question? — morpheus, Oct 15 '14 at 18:47

C# Singleton Thread Safety

2 Answers2