thread safe singleton pattern (as a property)

[Mountain Bikn' Guy]

Is the second version shown below better? I couldn't locate enough info
about [MethodImpl(MethodImplOptions.Synchronized)] in order to tell.

1.. My commonly used singleton pattern implementation looks like this (it
was inspired by Eric Gunnerson's book):

private static volatile MyClass singleton = null;
private static object sync = new object();//for static lock
public static MyClass Instance
{
get
{
if (singleton == null)
{
lock (sync)
{
if (singleton == null)
{
singleton = new MyClass ();
}
}
}
return singleton;
}
}//Instance

2. An alternate implementation inspired by 'Design Patterns Explained: A New
Perspective on Object-Oriented Design' by by Alan Shalloway, James R. Trott.

public static MyClass Instance
{
get
{
if (s_Instance == null)
{
DoSynchronizedExistenceCheck();
}
return s_Instance;
}
}

[MethodImpl(MethodImplOptions.Synchronized)]
private static void DoSynchronizedExistenceCheck()
{
if (s_Instance == null)
{
s_Instance = new MyClass();
}
}

 

[Jon Skeet [C# MVP]]

Is the second version shown below better? I couldn't locate enough info
about [MethodImpl(MethodImplOptions.Synchronized)] in order to tell.

It's the equivalent of doing:

lock (this)
or
lock (typeof(MyClass))

round the rest of the method, I believe. I don't believe it
particularly helps in this case.

The first version is *probably* safe due to MyClass being volatile, but
I wouldn't like to say for sure. However, there are fortunately simpler
and definitely reliable versions:

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

 

[Mountain Bikn' Guy]

Thanks Jon! Your page is very informative!
Mountain

Is the second version shown below better? I couldn't locate enough info
about [MethodImpl(MethodImplOptions.Synchronized)] in order to tell.

It's the equivalent of doing:

lock (this)
or
lock (typeof(MyClass))

round the rest of the method, I believe. I don't believe it
particularly helps in this case.

The first version is *probably* safe due to MyClass being volatile, but
I wouldn't like to say for sure. However, there are fortunately simpler
and definitely reliable versions:

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

 

[Mountain Bikn' Guy]

Jon,
You mention the following:
"There are complications if one static constructor invokes another which
invokes the first again."

I have run into this problem (or a very similar one) quite frequently,
actually, using the first singleton pattern I posted. Specifically, I have
seen cases where a threadsafe singleton created in a situation where the
static constructors which refer to each other in a cycle then seemingly
contains two different values in one field, depending upon whether that
field is accessed by the other singleton or some third object. Note that I
witnessed this even thought I don't explicity use a static constructor in my
singleton pattern.

Also, you mention "It still doesn't perform as well as the later
implementations" in regard to your 3rd example. However, I assume that
whenever you are mentioning performance on this page, you are referring only
to laziness. (I ready your excellent discussion on your "beforefieldinnit"
page, which led me to understand that laziness != better performance in all
situations -- and that also led me to ask you to clarify that the word
performance on your singleton page refers strictly to laziness).

Regards,
Mountain

 

[Jon Skeet [C# MVP]]

You mention the following:
"There are complications if one static constructor invokes another which
invokes the first again."

I have run into this problem (or a very similar one) quite frequently,
actually, using the first singleton pattern I posted. Specifically, I have
seen cases where a threadsafe singleton created in a situation where the
static constructors which refer to each other in a cycle then seemingly
contains two different values in one field, depending upon whether that
field is accessed by the other singleton or some third object. Note that I
witnessed this even thought I don't explicity use a static constructor in my
singleton pattern.

I may not have been very precise here - I'll check later on. You don't
need to be using an explicit static constructor to get this kind of
behaviour, just static initialization of some description.

Also, you mention "It still doesn't perform as well as the later
implementations" in regard to your 3rd example. However, I assume that
whenever you are mentioning performance on this page, you are referring only
to laziness. (I ready your excellent discussion on your "beforefieldinnit"
page, which led me to understand that laziness != better performance in all
situations -- and that also led me to ask you to clarify that the word
performance on your singleton page refers strictly to laziness).

Nope, it's not to do with laziness (and as you've inferred, enforcing
strict laziness can reduce performance). It's to do with how many
checks need to be done - with my code, the only check done on each call
is whether or not the type has been initialized (and the JIT can cache
that information, or pre-initialize if it's marked as beforefieldinit,
etc). With the double-check lock algorithm, you need to check whether
or not the static field is null every time, and the first time through
you need to take out a lock as well. You need to do those things *as
well* as all the checks that are in the faster version, because
obviously the type needs to still have been loaded/initialized (even if
initialization merely consists of making sure that there's no real
initialization to do).

I haven't got a handy benchmark on this ready to post, but I could
write one if you're interested.

 

[Mountain Bikn' Guy]

I may not have been very precise here - I'll check later on. You don't
need to be using an explicit static constructor to get this kind of
behaviour, just static initialization of some description.

Thanks. That clarification helps.

Nope, it's not to do with laziness (and as you've inferred, enforcing
strict laziness can reduce performance). It's to do with how many
checks need to be done.

Thanks again. I recognized the error of my statement -- but only after I had
already posted it. (Where's that "Unsend" button?)

Jon, I would really appreciate your clarification on whether the explicit
static ctor can be left out of both example 4 and example 5 of your
singleton patterns without impacting their thread safety.

Regards,
Mountain

 

[Jon Skeet [C# MVP]]

Jon, I would really appreciate your clarification on whether the explicit
static ctor can be left out of both example 4 and example 5 of your
singleton patterns without impacting their thread safety.

It definitely can. The static constructor itself is *only* necessary
for laziness.

 

[Miha Markic]

Hi Lefty

Thanks again. I recognized the error of my statement -- but only after I had
already posted it. (Where's that "Unsend" button?)

Actually there is one: Message/Cancel Message in our favorite newsreader OE.
Unfortunatelly it doesn't seem to work with ms newsgroups.

 

[Me]

Jon
Double locking pattern is not thread safe in .NET as well as in Java.
See Cris Brumme's blog and other places

 

[Jon Skeet [C# MVP]]

Double locking pattern is not thread safe in .NET as well as in Java.

Indeed, that's my feeling too. That's why I've said so in the article -
or rather, I've said that although there are reports that it *is*
thread-safe in .NET, I can't see why it would be, and other people
report that it isn't. Perhaps it's time to word that even more strongly
though...

 

[Mountain Bikn' Guy]

The original pattern I posted (#1, not #2) is based on Eric Gunnerson's
example. He shows that the use of the volatile keyword in this pattern makes
it threadsafe and efficient (the synchronization is only required if the
singleton has not been created).

The volatile keyword is what separates this example from the double locking
pattern you are discussing. (A write cannot be mored forward across a
volatile write. And a read cannot be moved backward across a volatile read.
In addition to precluding reordering, volatile also means that the JIT can't
keep the var in the register and that the var must be stored in global
memory on a multi-processor system.)

All that said, I believe Jon's patterns 4 & 5 are preferrable to (and better
performing than) this one that uses volatile -- but I have not yet seen any
indication that my Gunnerson-inspired pattern is not threadsafe. And Jon's
singleton page is now my favorite reference on this topic -- so Jon, please
keep adding to it as you can.

Regards,
Mountain

 

[Jon Skeet [C# MVP]]

The original pattern I posted (#1, not #2) is based on Eric Gunnerson's
example. He shows that the use of the volatile keyword in this pattern makes
it threadsafe and efficient (the synchronization is only required if the
singleton has not been created).

But the volatility makes it less efficient than the "thread safety via
class initialization" - every volatile read requires a memory barrier.

The volatile keyword is what separates this example from the double locking
pattern you are discussing.

Yup, I'm happy to believe that.

All that said, I believe Jon's patterns 4 & 5 are preferrable to (and better
performing than) this one that uses volatile -- but I have not yet seen any
indication that my Gunnerson-inspired pattern is not threadsafe.

No, I suspect it is - but it's complicated and less efficient. I can't
immediately see any benefit to it for "normal" singleton uses -
although there are rare cases I can imagine where it would be useful.

And Jon's
singleton page is now my favorite reference on this topic -- so Jon, please
keep adding to it as you can.

Aw, I'm touched

I'll make the thread-unsafety of the normal double-check algorithm
clearer and then add Eric thread-safe variant.

 

[Vinay Chaudhari]

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

Nice articule, although I have a doubt. CLR only guarantees that static
constructor will be called before any type access but it does not
guarantee the completion of the constructor. Indeed, I have seen some
unexpected errors due to this on one .NET CF application and to fix
bug, I just kick started the static constructor early by calling empty
static method on the class. I am just wondering whether this can cause
any weird scenarios in ur pattern (in case static constructor is long
running).

-Vinay.

 

[Jon Skeet [C# MVP]]

Nice articule, although I have a doubt. CLR only guarantees that static
constructor will be called before any type access but it does not
guarantee the completion of the constructor.

It guarantees that the static constructor will complete before any
other type access *unless* you're in the unusual situation where you
would get deadlock otherwise (eg one static constructor calls a method
in another type, whose static constructor calls a method back in the
first class). See section 9.5.3.3 of the ECMA CLI spec.

Indeed, I have seen some
unexpected errors due to this on one .NET CF application and to fix
bug, I just kick started the static constructor early by calling empty
static method on the class.

Could you give an example?

 

[Vinay Chaudhari]

It guarantees that the static constructor will complete before any
other type access unless you're in the unusual situation where you
would get deadlock otherwise (eg one static constructor calls a
method in another type, whose static constructor calls a method back
in the first class). See section 9.5.3.3 of the ECMA CLI spec.

Oh! Thanks for that. I was not aware of that. If I could remember
correctly I think I have read about it in Ritcher's Applied .NET
Framework programming where he has said that static constructor's
completion is not guaranteed because of possibility of deadlocks
arising due to cicular references amongs static constructors. My
mistake was that I assumed that the case applies evey time.

Could you give an example?

Sure, I was writing a base control that will put an icon in front of
containing control to indicate qualifying value for the data e.g.
blank, not documented, not applicatable or filled (not null). The icon
can be clicked to get the combo-box like panel (implemented as
singleton) to select one of blank/ND/NA values. Code goes something
like this...

Public Mustinherit Class MyControl : Inherits Control

Public Shared SomeVariable As ...

Shared Sub New()
'Static constructor
SomeVariable = <value>
End Sub

Public Sub New()
'constructor
...
<read access to the SomeVariable>
...
End Sub

Protected Overrides Sub OnPaint(e as PaintEventArgs)
'Paiting the control.
...
'Get icon from the StatusPanel class
icon = StatusPanel.GetIcon(CurrentStatus)
...
End Sub

'Nested class -> StatusPanel
'Implements singleton pattern
Private Class StatusPanel : Inherits Form

Private Shared _Instance As StatusPanel

Shared Sub New()
_Instance = New StatusPanel()
End Sub

Public Shared Function GetIcon(status As <Enumeration>) _
As Icon

'return the correct icon from the variable.
...
End Sub

'To popup the panel.
Public Shared Sub PopupPanel(...)
....
End Sub

...

Private SUb New()
...
'Load status icons from resources
'and store them into class variable
...
End Sub

...

End Class

End Class

In my main form, I was doing lot of intialization and also creating
controls (inherited from above control). I used to get NullException
error in the painting of the control because StatusPanel used to return
the null icon to it. However after form-load is complete, icons data
used to be present in the StausPanel i.e. to say that if I ignored the
exception, I could proceed ahead to get my expected behavior. If I used
debugger and tried to step-in, it will work correctly. To cure the
problem, I added blank static method "Load" to the StatusPanel class,
called it from staic method "Load" in Control class (because
StatusPanel was private nested class) and in begining of my form load
control intialization, I put call to this load method. Please note that
this application was for PocketPC and hence using .NET Compact
Framework.

-Vinay

 

[Jon Skeet [C# MVP]]

In my main form, I was doing lot of intialization and also creating
controls (inherited from above control). I used to get NullException
error in the painting of the control because StatusPanel used to return
the null icon to it. However after form-load is complete, icons data
used to be present in the StausPanel i.e. to say that if I ignored the
exception, I could proceed ahead to get my expected behavior. If I used
debugger and tried to step-in, it will work correctly. To cure the
problem, I added blank static method "Load" to the StatusPanel class,
called it from staic method "Load" in Control class (because
StatusPanel was private nested class) and in begining of my form load
control intialization, I put call to this load method. Please note that
this application was for PocketPC and hence using .NET Compact
Framework.

I'm still not exactly sure what your code was doing, to be honest -
there isn't enough of it to be certain. However, it's possible that
your main form initialising was triggering StatusPanel to be
initialised *during* that time, so the final parts of the
initialisation wouldn't have happened. As I say though, I can't be sure
without a complete example.