DEV Community

Discussion on: Design Patterns: Singleton

Collapse
 
gochev profile image
Nayden Gochev

I find this wrong and buggy, if two thread invoke the Instance at the same time there is a chance two variables to be created and in fact two different instances to be received by the two threads.

I am not a C# guy, sorry but usually in Java volatile and the double check locking helps I believe you can do the same in C#. Also I know in C# you have a Lazy<> which should be used ... but this pattern implementation is not good :( :(

Maybe there is some additional static magic that C# do, but I find it a bit smelly :D

I think the correct implementation is

public sealed class Singleton 
{
    private static readonly Lazy<Singleton>lazy = new Lazy<Singleton>(()=>new Singleton());

    public static Singleton Instance
    { 
        get 
        { 
            return lazy.Value; 
        } 
    }

    private Singleton() 
    {
    }
}

P.S. sorry for the wrong formatting (if it is wrong) I use { on the same line... :D:D:D so :D maybe I messed something :D

Collapse
 
gochev profile image
Nayden Gochev • Edited

oh yes also en.wikipedia.org/wiki/Double-check...

here you can check the proper implementation of your example

public class MySingleton {
    private static object myLock = new object();
    private static volatile MySingleton mySingleton = null; // 'volatile' is unnecessary in .NET 2.0 and later

    private MySingleton() {
    }

    public static MySingleton GetInstance() {
        if (mySingleton == null) { // 1st check
            lock (myLock) {
                if (mySingleton == null) { // 2nd (double) check
                    mySingleton = new MySingleton();
                    // In .NET 1.1, write-release semantics are implicitly handled by marking mySingleton with
                    // 'volatile', which inserts the necessary memory barriers between the constructor call
                    // and the write to mySingleton. The barriers created by the lock are not sufficient
                    // because the object is made visible before the lock is released. In .NET 2.0 and later,
                    // the lock is sufficient and 'volatile' is not needed.
                }
            }
        }
        // In .NET 1.1, the barriers created by the lock are not sufficient because not all threads will
        // acquire the lock. A fence for read-acquire semantics is needed between the test of mySingleton
        // (above) and the use of its contents. This fence is automatically inserted because mySingleton is
        // marked as 'volatile'.
        // In .NET 2.0 and later, 'volatile' is not required.
        return mySingleton;
    }
}

without lazy, az you can see volatile is NOT required now days, but double checked locking is !