4

We had a conversation (with coworkers) about why we needed to use Singleton for Controller, Business Services, and Repositories. The reason for this, they claim, is because singleton ensures that code is thread-safe. According to what I read in The Gang of Four, we should use Singleton when:

  • there must be exactly one instance of a class, and it must be accessible to clients from a well-known access point.
  • when the sole instance should be extensible by subclassing, and clients should be able to use an extended instance without modifying their code.

So For me it better to use Scoped and Transient services whenever possible and not Singleton.

For example I am using those services as Scoped

services.AddScoped<INonTailorMadeCostComputerService, NonTailorMadeCostComputerService>();
services.AddScoped<ICostComputerService, CostComputerService>();

They want to transform like this

services.AddSingleton<INonTailorMadeCostComputerService, NonTailorMadeCostComputerService>();
services.AddSingleton<ICostComputerService, CostComputerService>();

Am I missing something about using Singleton, Scoped for DI? Is it a good practice to use Singleton and not Scoped/Transient whenever is possible?

5
  • 2
    Unless you plan on spinning up more threads in a single request, I believe that ASP.NET requests are already thread-safe. Oct 19, 2021 at 14:55
  • 11
    "The reason for this, they claim, is because singelton ensures that code is thread-safe." We're not off to a good start since there's nothing about singletons that make them inherently thread-safe. There are quite a few questions concerning singletons, you may want to give them a look. Oct 19, 2021 at 15:01
  • 4
    Yes, I was about to say that. I would dispute the assertion that making something a singleton automatically confers thread safety. If anything, the opposite is probably true, since a singleton instance is almost certainly a shared instance. Oct 19, 2021 at 15:03
  • 2
    Caveat Emptor when changing the scope of a service. If a service is designed to be Scoped then it may not work correctly when you change the deployment to Singleton. Those bugs can be very subtle. Oct 19, 2021 at 17:35
  • You usually have a pattern that makes singleton creation threadsafe when followed. And usually you can make it threadsafe by protecting every function with a recursive lock. You can probably create a wrapper that makes it threadsafe. But on its own, a singleton isn’t threadsafe.
    – gnasher729
    Mar 9 at 20:10

5 Answers 5

9

Is it a bad practice to use X whenever possible?

Yes. It is bad practice to turn off your brain and lean on absolutes. It's far better to understand why.

So lets step back from that framework and try to find a more useful one.

The reason for this, they claim, is because singleton ensures that code is thread-safe.

Oh how wonderful it would be if we could just shove any arbitrary code into a framework and have it magically become thread safe. Sadly, that isn't what's happening here.

Transient objects are always different; a new instance is provided to every controller and every service.

Scoped objects are the same within a request, but different across different requests.

Singleton objects are the same for every object and every request.

Stack Overflow - AddTransient, AddScoped and AddSingleton Services Differences

What is happening is one copy of the object is being shared. Which actually sounds like it's less thread safe. But that also misses the mark.

Making a class thread safe is a lot more work than managing how it's created and how it's shared. There's also if it's mutable. There's if it's methods can be called concurrently.

So no. "singleton ensures that code is thread-safe" is a dangerous oversimplification.

So For me it better to use Scoped and Transient services whenever possible and not Singelton.

Don't know you or your needs but this is also sounds like a dangerous oversimplification. I'd feel better if you were saying things like: I have yet to find a need to ensure that every request gets the same copy of this object.

It's worth considering if you even need the code to be thread safe. Avoid exposing the code to multiple threads and you won't care if it's thread safe.

It's also worth understanding that what you're using here isn't the GoF's singleton pattern. You're using a library that named one of it's methods singleton. So be careful about applying too much of what the GoF wrote about the singleton pattern to this. They were looking at different code. Oh sure, the GoF ideas are just as valid as they ever were. But when you popularize a name for something it goes off and lives a life of its own.

8

Firstly, you are confusing singletons provided by a DI container with the "Gang of Four" singleton pattern. The latter is a glorified global variable and thus a complete anti-pattern and should never be used. Singletons provided by a DI container are almost totally unrelated. The only similarity being that you only have one of them in both cases, thus the common name "singleton".

Secondly, your co-workers are confused over thread safety. If I share a singleton that has state between threads, type safety is harder than giving each thread its own instance. So using singletons does not improve thread safety.

As to whether you should use singleton, scoped or transient for a type registration in the container depends entirely on the how the type is to be used:

  1. If you only ever want one instance for the entire duration of the lifetime of your web application (ie until the service is restarted), then use a singleton. Things like access to configuration details will fit this.
  2. If you want an instance that lasts for the duration of a user request (that eg might hold details of the user), then use scoped.
  3. If you want a new instance every time the container is asked to provide one (a resource access request that needs disposing quickly for example), then use transient.

If the particular type works with registering it as a singleton, then do so. As the container will only ever create one instance, this reduces the work that the garbage collector has to do. At the other end of the scale, transients put the biggest pressure on the heap, so should only be used when necessary. And scoped sits neatly in the middle.

7

Before you change whether a service is Transient, Scoped, or Singleton, do make sure you are not introducing bugs. When something is designed to be thrown away quickly, there might be assumptions that the code was built around. If you put that code in a new context where it wasn't designed to run you can pick up undefined behavior due to the new context.


The advantage of Transient and Scoped is that the instances are not shared across threads. You have instances that only persist for short periods of time, which prevents those instances from requiring locks. The advantages of this approach include:

  • Lockless design which prevents deadlocks or just lock contention
  • Only keeping objects in memory as long as they are needed
  • Reduced likelihood of memory leaks because the transient parent is destroyed as well.

The issue with Singleton objects is that you have to be very careful if you have persistent state. They are a common source of memory leaks due to a reference to data somewhere that won't be fully released. If you mutate persistent state, you have to lock your access to prevent race conditions. That increases the likelihood of lock contention or even deadlock--both of which are serious performance issues.

1

singelton ensures that code is thread-safe.

This is completely wrong. However, a singleton would be necessary to enforce thread safety for a shared resource, e.g. if you needed threads to synchronize themselves using a shared semaphore.

the sole instance should be extensible by subclassing, and clients should be able to use an extended instance without modifying their code.

These are good reasons to use a singleton over a static class. These days you should pretty much never be using static classes except for extension methods or possibly pure functions.

So For me it better to use Scoped and Transient services whenever possible and not Singelton.

A singleton is a good choice when

  • The class is either stateless or immutable
  • The class is stateful but the state can be shared safely and it makes sense to do so
  • The class semantically represents something of which there is only one, e.g. the application's configuration, or the O/S environment
  • An instance of the class is expensive to construct, so you only want to do it once
  • The class's dependencies are all singletons
3
  • 1
    The third bullet point is almost never true. For example you can't assume that there is only one CPU, GPU, etc. In the days of virtualization you can't even assume only one OS. Your first bullet point is very valid, but the are major caveats with the second bullet. Oct 19, 2021 at 17:12
  • Maybe I'm really far behind on virtualization, but as I understand it, there is a maximum of one ambient environment per process. There may be several environments, but they would all be running separate processes, each with their own set of singleton instances. I agree that an environment may have more than one CPU or GPU.
    – John Wu
    Oct 19, 2021 at 17:15
  • It has just been my experience that whatever assumptions led to the choice of a singleton were never true in the long run--and caused bugs when it changed. Oct 19, 2021 at 17:33
1

To put it mildly, what these engineers are proposing, is pure cancer.

In an effort to make the data repository and business services thread-safe by making every one of them singletons, the engineers have actually created vulnerabilities where threading problems and side effects can be easily introduced into the application.

Using AddScoped each request get's it's own copy of the service, but using AddSingleton the DI framework creates a single instance of the service and shares it across all requests. This means that each HTTP request that comes in will use the same instance of the service, and any data stored in that instance will be accessible to all requests, thus creating thread safety issues where otherwise there would have been none.

Using this approach means every function, library, and service utilized throughout the application must be evaluated and tested for thread safety and side effects, including race conditions and deadlock scenarios. Additionally if the repository or business services even share any transient services, then thread safety can become an issue even between separate services.

Since singletons persist in memory for the lifetime of the application, this approach has also made the application susceptible to memory leaks, now it is every developers responsibility to make sure all objects are out of reference and cleaned up before exiting or they may persist in memory, growing with every request.

This approach has also decreased the applications testability and reliability, as testing multi-thread scenarios is more complex and challenging.

From James Michael Hickey's blog post discussing how singletons utilizing transitive services can allow completely different HTTP requests to add, remove and modify elements of each others collections:

https://www.blog.jamesmichaelhickey.com/NET-Core-Dependency-Injection/

Singletons + Transitive Services

A singleton, again, lives “forever”. It’s always the same instance.

Transitive services, on the other hand, are always a different instance when requested - or resolved.

So here’s an interesting question: When a singleton depends on a transitive dependency how long does the transitive dependency live?

The answer is forever. More specifically, as long as it’s parent lives.

Since the singleton lives forever so will all of it’s child objects that it references.

This isn’t necessarily bad. But it could introduce weird issues when you don’t understand what this setup implies.

Thread-Safety Perhaps you have a transitive service - let’s call it ListService that isn’t thread-safe.

ListService has a list of stuff and exposes methods to Add and Remove those items.

Now, you started using ListService inside of a singleton as a dependency.

That singleton will be re-used everywhere. That means, on every HTTP Request. Which implies on many many different threads.

Since the singleton accesses/uses ListService, and ListService isn’t thread-safe - big problems!

Be careful.

A strong case should be made for using a singleton, each case should be evaluated thoroughly before making the decision, there should be compelling reasons for its usage. A robust and well-supported argument must be presented in favor of utilizing a singleton in any given scenario.

If you want to constantly be putting out fires and want your application to be the talk of the organization, then by all means, use this approach.

Not the answer you're looking for? Browse other questions tagged or ask your own question.