Thread Safe C# Singleton Pattern

Thread Safe C# Singleton Pattern

Performing the lock is terribly expensive when compared to the simple pointer check instance != null.

The pattern you see here is called double-checked locking. Its purpose is to avoid the expensive lock operation which is only going to be needed once (when the singleton is first accessed). The implementation is such because it also has to ensure that when the singleton is initialized there will be no bugs resulting from thread race conditions.

Think of it this way: a bare null check (without a lock) is guaranteed to give you a correct usable answer only when that answer is "yes, the object is already constructed". But if the answer is "not constructed yet" then you don't have enough information because what you really wanted to know is that it's "not constructed yet and no other thread is intending to construct it shortly". So you use the outer check as a very quick initial test and you initiate the proper, bug-free but "expensive" procedure (lock then check) only if the answer is "no".

The above implementation is good enough for most cases, but at this point it's a good idea to go and read Jon Skeet's article on singletons in C# which also evaluates other alternatives.

Singleton and Thread Safety?

  1. First I would start by adapting your Singleton to use the Lazy<T> class in the .NET library. "Lazy<T> provides support for lazy initialization" as the MSDN docs state, and it also provides a bool parameter isThreadSafe which when true "makes the instance usable concurrently by mutliple threads".

    private static Lazy<AppSession> _instance = 
    new Lazy<AppSession>(
    () => new LazyAppsession(), isThreadSafe: true
    );
  2. Secondly I would mark the Lazy<AppSession> as readonly as well, that way no method or code can overwrite the value of the _instance field.

  3. As for the ActionExecuted method, I don't know whether you'd still need to make that one thread safe by locking it... It's something that depends I guess, but don't take my word for it - I hope other can extend my answer on that - as I'd like to know myself as well!

Thus your class would then look something like this - but I'm not sure on the ActionExecuted method, and as I said, the only person I know who can make sense of that would be @JonSkeet.

public class AppSession() {
private static readonly Lazy<AppSession> _instance =
new Lazy<AppSession>(
() => new LazyAppsession(), isThreadSafe: true
);
public static AppSession Instance {
get { return _instance.Value; }
}

private AppSession() { }

private string _actionName = "none";
private DateTime? _actionTime = null;
public void ActionExecuted(string action) {
_actionName = action ?? String.Empty;
_actionTime = DateTime.UtcNow;
}
}

Can a singleton initialize thread-safely just use public static field rather a Lazy T or other Singleton solution?

Fundamentally, you are seeing behaviour that isn't guaranteed. Because it isn't guaranteed, you can't rely on it. Take the below code as an example. What output will it generate? The answer is it depends.

17.4.5.1: "If a static constructor (§17.11) exists in the class, execution of the static field initializers occurs immediately prior to
executing that static constructor. Otherwise, the static field
initializers are executed at an implementation-dependent time prior to
the first use of a static field of that class."

implementation-dependent means multiple orders are valid, as long as the rules are kept.

It might generate:

The Dictionary has been initialized now
Start Running...
Start Running...

Or it might generate:

Start Running...
Start Running...
The Dictionary has been initialized now

You can validate it yourself at https://dotnetfiddle.net/d5Tuev (switch runtimes on the left side).

Now, in your testing you have experienced one of those behaviours. But that behaviour isn't guaranteed, and thus you shouldn't rely on it.

using System.Collections.Concurrent;
using System.Threading;

namespace Samples.Console
{
public class Program
{
public static void Main(string[] args)
{
Thread thread1 = new Thread(() => StaticField.StaticFieldTest());
Thread thread2 = new Thread(() => StaticField.StaticFieldTest());

thread1.Start();
thread2.Start();

Thread.Sleep(1000);
}

public class StaticField
{
public static readonly ConcurrentDictionary<string, string> _dic = InitDictionary();

public static void StaticFieldTest()
{
System.Console.WriteLine("Start Running...");

_dic.TryAdd(string.Empty, string.Empty);
_dic.TryAdd(string.Empty, string.Empty);
_dic.TryAdd(string.Empty, string.Empty);
}

public static ConcurrentDictionary<string, string> InitDictionary()
{
System.Console.WriteLine("The Dictionary has been initialized now");
Thread.Sleep(500);
return new ConcurrentDictionary<string, string>();
}
}
}
}

In addition - Lazy or LazyWithNoExceptionCaching are quite useful in some contexts - e.g. if you have an object which takes 20 seconds to create, but you want to ensure you create only one of them.

What are the dangers of non thread safe singleton?

Since the singleton pattern restricts the class to only one instance, your approach violates that pattern.

If for any reason this is ok for your implementation, and we follow your stated example, there is a very high chance that each of the two concurrent accesses will get another instance of ThreadNotSafeSingleton. This may happen due to optimization if the compiler decides that there is no need to read back the _instance variable it just wrote before returning. This optimization behaviour is defined by the memory model of your C# implementation.

The volatile keyword is often cited as a possible solution, but it will not solve the synchronization issue (as pointed out by BionicCode), when thread 1 passes the if (_instance == null)line and gets put to sleep, then thread 2 also evaluates the same if and gets to instanciate the singleton. When thread 1 wakes up later, it will then instanciate another singleton.

Thread safe singleton with async operation

Based on this post, a lot of people appear to advocate just using LazyCache. Making the implementation:

public class ExampleService {

private readonly IAppCache cache;

public ExampleService(IAppCache cache) {
this.cache = cache;
}

private async Task<string> GetMessageFromServerAsync() {
// result = async stuff
return result;
}

public async Task<string> GetMessageAsync() {
return await cache.GetOrAddAsync("MessageKey", GetMessageFromServerAsync);
}

}

Thread safe Singleton: why the memory model does not guarantee that the new instance will be seen by other threads?

Can you please explain why doesn't the memory model does not guarantee that the new value of instance will be seen by other threads?

The memory model is complex and not terribly clearly documented at the moment, but fundamentally there are very few situations where it's safe to rely on a value that's written by one thread being "seen" on another thread without either some locking or other inter-thread communication going on.

For example, consider this:

// Bad code, do not use
public class BigLoop
{
private static bool keepRunning = true;

public void TightLoop()
{
while (keepRunning)
{
}
}

public void Stop()
{
keepRunning = false;
}
}

If you created two threads, one of which calls TightLoop and another of which calls Stop, there's no guarantee that the looping method will ever terminate.

There are lots of levels of caching in modern CPUs, and requiring that every read goes back to main memory would remove a lot of optimizations. So we have memory models which make guarantees about which changes will definitely be visible in what situations. Other than those guarantees, the JIT compiler is allowed to assume that there's effectively only a single thread - so it could cache the value of the field in a register, and never hit main memory again, for example.

The currently-documented memory model is woefully inadequate, and would suggest that some clearly-weird optimizations should be valid. I wouldn't go too far down that route, but it's worth reading Joe Duffy's blog post on the CLR 2.0 memory model. (That's stronger than the documented ECMA memory model, but a blog post isn't the ideal place for such a key piece of documentation, and I think more clarity is still needed.)

The static variable is located on the heap, but why it is not shared with other threads?

It is shared with other threads - but the value won't necessarily immediately be visible.



Related Topics



Leave a reply



Submit