Singleton Question

[Chris Fink]

I am trying to implement a singleton pattern in my application. I need to
make sure that this class is both thread safe and ensure that only 1 instance
can created at any given time.

As you can see below I create a two instances of the singleton class, s1 and
s2. Both s1 and s2 are indeed the same instance as I expected. However,
when s2 changes the Count to 2, the count for S1 also changes to 2!!! This is
the part I am confused about since I would expect this singleton to be thread
safe....the fact that the s2 instance changed the value in s1 will cause
problems in a multithreaded app. Am I missing something here? How do I
prevent s2 and subsequent instances making changes to my singleton while s1
has a lock on it?

==== my singleton class ====
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

class Singleton
{
private static Singleton instance;

// Note: Constructor is 'protected'
protected Singleton()
{
}

public int Count { get; set; }

public static Singleton Instance()
{
lock(typeof(Singleton))
{
// Use Lazy initialization
if (instance == null)
instance = new Singleton();
lock(instance)
{
return instance;
}

}
}
}

==== test code ====
// Constructor is protected -- cannot use new
Singleton s1 = Singleton.Instance();
s1.Count = 1;
Singleton s2 = Singleton.Instance();
s2.Count = 2;

int i = s1.Count; //2nd instance changed my s1 count? I thought
locking would prevent this?

if (s1 == s2)
{
Console.WriteLine("Objects are the same instance");
}

// Wait for user
Console.Read();

 

[Philippe Leybaert]

Locking only prevents simultaneous access to a variable by two threads.

What you're doing here is creating two variable s1 and s2 that are actually
pointing to the same object (that's actually the whole idea of having a
singleton object). So when you change a member of s1, you're also changing
the object where s2 points to, because s1 and s2 point to the same object
("the singleton").

-Philippe

 

[Chris Fink]

I understand. Perhaps I am looking for a different design pattern then. If
my requirements are to only allow 1 instance at a time and prevent other
threads from changing that instance while it is in use, what would I use?
Essentially, I would be putting all the requests/threads in a line and only
allow 1 thread to instantiate and do its work while the others wait in line
for it to become available.

Thanks for your help

 

[Philippe Leybaert]

You would have to place a lock on the object itself before accessing it from
different threads:

Like this:
Singleton mySingleton = Singleton.Instance();

lock (mySingleton)
{
s1.Count = 2;

// as long as you stay inside the
// locked section, s1.Count is
// guaranteed to stay the same
}

-Philippe

 

[Chris Fink]

Thank You.

I guess this means there is no way to make the singleton thread safe, but
instead you need to make the singleton instance thread safe. This will work
as long as all future consumers of this class are clear it is not a thread
safe class.

 

[Family Tree Mike]

Jon Skeet had a recent post about making a "thread singleton" object through
an attribute (I think). I cannot find the post again, and cannot remember
the keyword. I'm sure he will recall and post. Perhaps this is the solution
for you.

 

[Peter Duniho]

I understand. Perhaps I am looking for a different design pattern then.

Just judging from the sample code you posted, I think so. If anything,
I'd say that the singleton pattern is the opposite of what you want.

If my requirements are to only allow 1 instance at a time and prevent
other
threads from changing that instance while it is in use, what would I use?

Your example doesn't show multiple threads trying to change the object at
the same time. Instead, it performs all of the changes in a single thread.

As far as what your example does show, there's not really anything you can
do to prevent that. You seem to want there to only be a single variable
referencing an instance of a class, but no matter what you do, you could
always just assign the reference found in one variable to another
variable. There's no way to detect that scenario from within the class,
nor do I think it makes any sense at all to try to prevent it.

Now, let's assume you had instead posted an example where the same object
is in fact used from multiple threads. It _is_ possible to make a class
thread safe, but without knowing more about what exactly you're trying to
do, it's hard to advise you.

One of my favorite ways to deal with thread safety is to make a class
immutable. But this only applies in certain situations. Another way is
to make sure that any method that can mutate the class is protected
internally with a lock() statement. This can be a good solution, but it
may involve writing a lot of lock() statements and can incur some
performance penalty, especially when there is actually some contention for
the object.

Essentially, I would be putting all the requests/threads in a line and
only
allow 1 thread to instantiate and do its work while the others wait in
line
for it to become available.

This approach is antithetical to the whole point of multithreading. Why
would you want to have multiple threads that intentionally serialize
themselves on the same object? You might as well just have a single
thread use the same object for multiple iterations of the task.

Again, a better-elaborated explanation of what you're really trying to do
would go a long way.

Pete

 

[Martin Bonner]

It _is_ possible to make a class
thread safe, but without knowing more about what exactly you're trying to
do, it's hard to advise you.

One of my favorite ways to deal with thread safety is to make a class
immutable. But this only applies in certain situations.

.... but when it does apply it's the way to go!

Another way is
to make sure that any method that can mutate the class is protected
internally with a lock() statement. This can be a good solution, but it
may involve writing a lot of lock() statements and can incur some
performance penalty, especially when there is actually some contention for
the object.

Can I inject a word of caution here? It is not sufficient to have a
lock on all the mutating functions - you need to make sure that all
the mutations you need to make are provided by member functions which
take a lock.

For example, if you have a simple integer property whose setter
function locks, it is /not/ possible to reliably increment the
property. The compiler will turn:
var.Property++;
into (in pseudo code)
int temp = var.get__Property();
temp++;
var.set__Property(temp);

The call to set_Property will grab the lock all right, but that is too
late. We really needed to grab the lock /before/ the call to the
getter!

(I am pretty sure that Peter knows this already, but others may not
and just slapping locks on the mutators is a superficially attractive
solution.)

Again, a better-elaborated explanation of what you're really trying to do
would go a long way.

Seconded!

 

[Jon Skeet [C# MVP]]

Can I inject a word of caution here? It is not sufficient to have a
lock on all the mutating functions - you need to make sure that all
the mutations you need to make are provided by member functions which
take a lock.

For example, if you have a simple integer property whose setter
function locks, it is /not/ possible to reliably increment the
property. The compiler will turn:
var.Property++;
into (in pseudo code)
int temp = var.get__Property();
temp++;
var.set__Property(temp);

The call to set_Property will grab the lock all right, but that is too
late. We really needed to grab the lock /before/ the call to the
getter!

Not only that - we need to *keep* the lock while we increment. Adding a
lock to the getter isn't good enough, as two threads could both get
(acquiring and releasing the lock), then both increment, then both set
(acquiring and releasing the lock). You'd only "see" one increment as a
result.

 

[Martin Bonner]

Not only that - we need to *keep* the lock while we increment. Adding a
lock to the getter isn't good enough, as two threads could both get
(acquiring and releasing the lock), then both increment, then both set
(acquiring and releasing the lock). You'd only "see" one increment as a
result.

Yes, that was exactly what I meant (grab the lock before we get the
value, and keep hold of it until after we have finished setting).
Thanks for clarifying.

 

[Peter Duniho]

Can I inject a word of caution here? It is not sufficient to have a
lock on all the mutating functions - you need to make sure that all
the mutations you need to make are provided by member functions which
take a lock. [...]

Yes, sorry...my post was vaguely written. Thanks for the clarification.

Pete

 

[sainath thota]

public sealed class singelton
{
private static readonly singelton instance;
private static Object syncroot=new Object();
private singelton();
public static singelton instance
{
get
{

if(instance==null)
{
Lock(syncroot)
{
if(instance==null)
instance=new singleton();
}
}return instance;

}
}
}

 

[Marc Gravell]

There is a very good discussion of Singleton patterns on Jon's site:
http://www.pobox.com/~skeet/csharp/singleton.html

Note that this double-checked/locked approach is exactly the 3rd
version, which Jon then justifies as "// Bad code! Do not use!"

Marc

 

[Mythran]

There is a very good discussion of Singleton patterns on Jon's site:
http://www.pobox.com/~skeet/csharp/singleton.html

Note that this double-checked/locked approach is exactly the 3rd version,
which Jon then justifies as "// Bad code! Do not use!"

Marc

One of the comments Jon writes in the 2nd bullet of the 3rd version:

"I tend to try to avoid situations where experts don't agree what's right
and what's wrong!"

Hmm, Jon = expert....does he not agree either/or? Or is he not an expert in
the 'explicit memory barrier' field?

Mythran

 

[Marc Gravell]

Or is he not an expert in

the 'explicit memory barrier' field?  

I think the key point here is that there are people who truly are
experts in this area (far deeper than language level) who can't agree;
it doesn't help that part of this is theory (the CLR/CLI spec) rather
than practice (the MS .NET CLR implementation), which means you can't
strictly prove it just by example, because that could be an
implementation detail. So since it is so easy to work around the
ambiguity...?

Marc

 

[Jon Skeet [C# MVP]]

One of the comments Jon writes in the 2nd bullet of the 3rd version:

"I tend to try to avoid situations where experts don't agree what's right
and what's wrong!"

Hmm, Jon = expert....does he not agree either/or? Or is he not an expert in
the 'explicit memory barrier' field?

I believe that the version with a volatile field is probably okay.
However, I don't regard myself as a real expert in this field (or
virtually any field, to be honest). When people who definitely know
more than I do disagree, it makes life tricky.

I *think* that the volatile version is largely regarded as safe, but
there are lock-free ways of doing it which are more debatable.