Singleton class with Dependency Injection c#

Question

We have an external project with a QCServiceLog class that has a ILogging dependency which is resolved by Unity. But QCServiceLog is a Singleton class as you can see in the following example:

private readonly ILogging _logging = null;

private static QCServiceLog _instance = null;
public static QCServiceLog Instance
{
    get
    {
        return _instance;
    }
}

public QCServiceLog(ILogging logging)
{
    _logging = logging;
    if (_instance == null)
    {
        _instance = this;
    }
}

We are trying to use it, and in our solution we did the registration like:

uc.RegisterType<ILogging, QCFileManager>(new ContainerControlledLifetimeManager());

But since QCServiceLog is a Singleton we believe that the code never gets through the constructor, then _instance is never instantiated. We are using it doing this:

QCServiceLog.Instance.Log(ex);

Is that Singleton implemented correctly? We believe that it´s never doing a new of QCServiceLog.

What do you think? Is there anything that we can do without changing the external project? The exception as you can imagine is:

Object reference not set to an instance of an object.

I would really appreciate your help!

Dave Black · Accepted Answer · 2024-01-05 21:19:18Z

5

The code you've provided is not thread-safe - have a look here Implementing the Singleton Pattern in C#.

The thread-safety issue shouldn't be a problem as long as the following are always followed:

QCServiceLog is always registered as a Singleton (which you appear to be doing).
There is no code that manually creates an instance of QCServiceLog. Since your constructor is public, it is possible.

As a matter of implementing a pattern correctly, consider the code below as one of several possible alternatives to resolve the thread-safety. In general, there are preferred alternatives (see the link I mention above) but I suggest this one given your situation using DI. This will at least guarantee your singleton's _logging instance is not overwritten by a manual instance creation:

private static readonly object _syncLock = new object();

public QCServiceLog(ILogging logging)
{
    if (_logging == null)
    {
        lock(_syncLock)
        {
            if (_logging == null)
            {
                _logging = logging;
            }
        }
    }

    if (_instance == null)
    {
        lock(_syncLock)
        {
            if (_instance == null)
            {
                _instance = this;
            }
        }
    }
}

One other issue is that the constructor is not hidden. Thus, even if your IoC container registers the class as a Singleton, an instance of this class can still be constructed anywhere in the code using the new operator. I found a memory leak in an app I worked on caused by this very thing (intention was to use via DI/IoC which was registered as a singleton but was newed up by code manually.

edited Jan 5, 2024 at 21:19

answered Jul 27, 2017 at 22:15

Dave Black

8,1992 gold badges58 silver badges47 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Scuba Steve Over a year ago

This singleton pattern is still not thread safe. The link you provided does demonstrate thread safe singleton patterns, but the one you have demonstrated here is not thread safe. What if two threads are inside if(_instance == null) at about the same time?

Dave Black Over a year ago

@ScubaSteve - sorry for the confusion. I was referring to the OP's code, not specifying how to correct it. I've replaced the OP's code with a possible alternative suitable for his situation.

Jeroen · Accepted Answer · 2023-10-06 07:39:52Z

Is that Singleton implemented correctly?

Yes. But it needs to be instantiated first from the container before you can get a static instance of it.

In other words, you can't use this line:

QCServiceLog.Instance.Log(ex);

until you first call:

container.Resolve<QCServiceLog>();

(assuming you have registered an instance of ILogging with Unity) because the first call will not hit the constructor to populate the _instance value.

You are probably better off either injecting it directly where it is needed:

public class Foo(QCServiceLog log)

Or alternatively making a wrapper class that contains the logic to wire it up correctly before using it that is injected via your DI container.

Either way, you should note that DI is primarily for your application. 3rd party utilities don't necessarily do things in a DI-friendly way, so you may need to use a facade or adapter in order to make them DI-friendly for use within the context of your application.

Collectives™ on Stack Overflow

Singleton class with Dependency Injection c#

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related