.Net singleton pattern multi-threading issue...

I came across this today in some code i am reviewing ... and thought i would share it with everyone as i am sure not many people know about this...

In the singleton pattern you have a static member variable that 'stores' your single instance of whatever you are giving out.

Most code that i review does it like this:

E.g.

public class Yada
{
private static MyFunkyObject _foo;

    public static MyFunkyObject Foo
{

        get
{

           if(_foo == null)
_foo = new MyFunkyObject();

return _foo;
        }
    }
}

Now ... this wont work. Why? Because it is not threadsafe. You 'could' potentially have two threads come in and both get past the if(_foo == null) statement ... and then both go and set _foo up.

This breaks your singleton pattern. Most coders who i point this out to don't even realize they have this issue.

To make your singleton threadsafe you need to do a couple of things. Put a Mutex or Critical section around access to _foo. In C# you have the luxury of using the 'lock' keyword to do this. This gives you a nice threadsafe singleton ... but your code takes a performance hit.

E.g.

public class Yada
{
private static MyFunkyObject _foo;
private Object _synclock = new Object();

public static MyFunkyObject Foo
{

get
{

lock(_synclock)
{
if(_foo == null)
_foo = new MyFunkyObject();
}

return _foo;
}
}
}

There is an even nicer way to do this:

public class Yada
{
private static MyFunkyObject _foo = new MyFunkyObject();

public static string Foo
{
get
{
return _foo;
}
}
}

I have seen many peoples Singleton implementations coded poorly as per the first example. Maybe most people are developing and testing on a single proc machine and don't see this happen ... but you could quite easily be having these issues when you deploy onto that nice beefy 4 proc web server.

(I think i have got this right ... but if please correct me if i am wrong!)