Thread safety ??

This doubt is regarding synchronisation question in Singleton pattern code
of C#

I had created a class as

public sealed class SecuriteManager
{
private static volatile SecurityManager instance;
private static object syncRoot = new Object();

private SecurityManager () { }

public static SecurityManager GetInstance
{
get{
if(null == instance){
lock(syncRoot){
if (instance == null)
instance = new SecurityManager ();
}
}
return instance;
}
}
Any reason to use this complicated pattern rather than a simple one as
specified on

http://www.pobox.com/~skeet/csharp/singleton.html

?
public bool IsAllowed(strin g UserName)
{
//A very long process here.
//For example access to a webservice which might take 3 seconds.
//HttpContext.Cur rent.Session[UserName] = returnValueFrom Webservice;
}
}
Now when 2 users access IsAllowed at the same time, is the process thread
safe ?

This doubt is regarding synchronisation question in Singleton pattern code
of C#

I had created a class as

public sealed class SecuriteManager
{
private static volatile SecurityManager instance;
private static object syncRoot = new Object();

private SecurityManager () { }

public static SecurityManager GetInstance
{
get{
if(null == instance){
lock(syncRoot){
if (instance == null)
instance = new SecurityManager ();
}
}
return instance;
}
}

public bool IsAllowed(strin g UserName)
{
//A very long process here.
//For example access to a webservice which might take 3 seconds.
//HttpContext.Cur rent.Session[UserName] = returnValueFrom Webservice; }
}
Now when 2 users access IsAllowed at the same time, is the process thread
safe ?
Or should I use lock for each call in my function as

public bool IsAllowed(strin g UserName)
{
lock(syncRoot)
{
//A very long process here.
//For example access to a webservice which might take 3 seconds.
//HttpContext.Cur rent.Session[UserName] = returnValueFrom Webservice; }
}

Please suggest.

This doubt is regarding synchronisation question in Singleton pattern code
of C#

I had created a class as

public sealed class SecuriteManager
{
private static volatile SecurityManager instance;
private static object syncRoot = new Object();

private SecurityManager () { }

public static SecurityManager GetInstance
{
get{
if(null == instance){
lock(syncRoot){
if (instance == null)
instance = new SecurityManager ();
}
}
return instance;
}
}

public bool IsAllowed(strin g UserName)
{
//A very long process here.
//For example access to a webservice which might take 3 seconds.
//HttpContext.Cur rent.Session[UserName] = returnValueFrom Webservice; }
}
Now when 2 users access IsAllowed at the same time, is the process thread
safe ?
Or should I use lock for each call in my function as

public bool IsAllowed(strin g UserName)
{
lock(syncRoot)
{
//A very long process here.
//For example access to a webservice which might take 3 seconds.
//HttpContext.Cur rent.Session[UserName] = returnValueFrom Webservice; }
}

Please suggest.

>private static volatile SecurityManager instance;

In addition to the other comments, should you wish to continue with a
singleton class then the volatile keyword is not needed.

Yes it is - otherwise the double-checked locking he's got isn't thread-

Phil Jenson <ph**@REMOVETHI S.jenson.co.uk> wrote:

>private static volatile SecurityManager instance;

In addition to the other comments, should you wish to continue with a
singleton class then the volatile keyword is not needed.

Yes it is - otherwise the double-checked locking he's got isn't thread-
safe. It's not the nicest way of achieving thread-safety in the first
place, but at least it *is* safe at the moment.

--
Jon Skeet - <sk***@pobox.co m>
http://www.pobox.com/~skeet
If replying to the group, please do not mail me too

Didn't we conclude before that even with volatile this may not be safe?
I don't *think* so.
The null check is still disturbing as I think thread2 can see a "not" null ref
and the instance still not fully constructed by thread1 yet, and possibly
other complicated scenarios that I have forgot.
It shouldn't do - the write to the volatile variable should have made
sure that everything's been fully constructed before thread2 can see
it.
As there was still some
question on this and CLR memory model, I thought explicit lock (then check)
or static construction were the safe ways for now? Or has someone proved
this works in all cases?

sure that everything's been fully constructed before thread2 can see
it.