Thread-safe Singleton in C#

This has two parts. Please read first part first and think and then read second part.
+ First part:
Is this a thread-safe singleton class:

1 public class SingletonClass

2 {

3 private SingletonClass () { Constructor (); }

4 static SingletonClass instance;

5 public static SingletonClass Instance

6 {

7 get

8 {

9 lock (typeof (SingletonClass))

10 {

11 if (instance == null)

12 instance = new SingletonClass ();

13 lock (instance)

14 {

15 return instance;

16 }

17 }

18 }

19 }

20 void Constructor () { }

21 }

--------------------------------------------------------------------------------

+ Second part:
I think this one is a thread-safe singleton:

1 public class SingletonClass

2 {

3 public static object Hey_IAmStillUsingThisInstance_Lock = new object ();

4 private SingletonClass () { Constructor (); }

5 static SingletonClass instance;

6 public static SingletonClass Instance

7 {

8 get

9 {

10 lock (typeof (SingletonClass))

11 {

12 if (instance == null)

13 instance = new SingletonClass ();

14 lock (instance)

15 {

16 return instance;

17 }

18 }

19 }

20 }

21 void Constructor () { }

22 public void DoSomthing () { }

23 }

Which should be used this way:

100 static void Main(string[] args)

101 {

102 lock (SingletonClass.Hey_IAmStillUsingThisInstance_Lock)

103 {

104 SingletonClass.Instance.DoSomthing ();

105 }

106 }

As you see it is a very very in-the-way kind of mistake! I did not beleive it when I came across it first time! The first one is thread safe just at the moment it's refrence is accessed by 'Instance' property not after that.
This filth is shared memory.
>:(

I'm assuming this is C#

And it's more a C# related question than it is a PL related question - hence being not the kind of topic that LtU invites.

That said, using lock(typeof(MyClass)) is frowned upon in C#.

And it does work, at least most of the time. But there are problems with it: First, getting the type object is actually a fairly slow process (although most programmers would guess that it's extremely fast); second, other threads in ANY CLASS and even different programs running in the same application domain have access to the type object, so it's possible that they'll lock the type object on you, blocking your execution entirely and causing you to hang.

The basic problem here is that you don't own the type object, and you don't know who else could access it. In general, it's a very bad idea to rely on locking an object you didn't create and don't know who else might be accessing. Doing so invites deadlock. The safest way is to only lock private objects.

By Chris Rathman at Wed, 2007-08-22 15:20 | login or register to post comments

What language are you using?

You need to think about the language you are using, don't just blindly implement the design pattern the same way you would in any other language.

In C#, a singleton is simply a static class.

public static class SingletonClass {

}

If you need to initialize anything, include this line.

    private _Data = InitData();

You could also use a static constructor, but this design is perfered.

By Jonathan Allen at Wed, 2007-08-22 17:05 | login or register to post comments

Does this really work?

(I don't know C# very well; my observations here are based on the MSDN documentation for static classes.)

A singleton is an object. As far as I can tell, you can't get an object out of a C# static class declaration. Can you?

// A sorting function
void Sort(Thing[] array, Comparator comparator) { ... }

// The singleton comparator
static class MyComparatorSingleton impelements Comparator
{
   public static int Compare(Thing a, Thing b) { ... }
}

// Call to the sorting function with the singleton.  Does this work?
Sort(array, MyComparatorSingleton)

With the conventional singleton pattern, you would do:

Sort(array, MyComparatorSingleton.getInstance()

By Kannan Goundan at Wed, 2007-08-22 21:05 | login or register to post comments

Links

Recently, Derek Elkins suggested that forum topics should contain at least one relevant link. This is a case which would have benefited from that: Googling for singleton c# turns up a number of apparently good summaries of the subject. Aside from the questionable general PL relevance, it's not clear that anything is being raised here that hasn't previously been covered, in more detail, elsewhere.

By Anton van Straaten at Thu, 2007-08-23 01:12 | login or register to post comments

Did I missed the point?

1 - I thought this is about Singleton pattern and thread-safety; which in my opinion is not a just-C# thing. If you think I did wrong - Chris Rathman - then I am sorry.
2 - Dear Jonathan Allen; please carefully review the process of accessing the singleton instance. Here the pointer to the instance and just the pointer to it is thread-safe not the whole object. Assume the singleton instance has a property (a public field).
In two separate thread we have these lines:
i) [Singleton.Instance].Value = new Thing();
ii) [Singleton.Instance].Value = new Thing();
The part in brackets is thread safe. But the assigning part is not! So actually 1st version is useless.
Getting the reference is thread-safe which is the part in brackets. But calling methods or using properties is not.

By kaveh.shahbazian at Sat, 2007-08-25 10:51 | login or register to post comments

Lambda the Ultimate

User login

Navigation