CODEWITHSUNDEEP
c#thread-safety
morpheus
morpheus

Reputation: 20382

C# Singleton Thread Safety

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).

Upvotes: 3

Views: 539

Answers (2)

Los Frijoles
Los Frijoles

Reputation: 4821

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..
}

Upvotes: 1

AtinSkrita
AtinSkrita

Reputation: 1453

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() { }
}

Upvotes: 1

Related Questions