Design Patterns: Singleton

Sign in to queue

Description

This is the fifth of an eight part series where Robert is joined by Phil Japikse to discuss design patterns. A design pattern is a best practice you can use in your code to solve a common problem.  In this episode, Phil demonstrates the Singleton pattern. This pattern restricts the instantiation of a class to one instance and provides global access to that instance.

Embed

Download

The Discussion

  • User profile image
    DonaldA

    When I started with VB 1.0, I found it so cool that I could drag drop UI components unto a form and in a short time have a very useful program.
    What I would like to see on the next Toolbox is the UI controls that I can drag drop Paint 3D objects unto 3D form and use on my desktop, phone or Mixed Reality headset.  I do not want to have to learn Unity.
    I know this does not exist yet, but someone should get to work on it really quick.  Seems Apple with their Ark kit is in the lead.

  • User profile image
    wkempf

    This has been a great series so far, but we just went off the rails. Figures we'd do so with the "anti-pattern" that is the Singleton. :)

    First, I want to clear up a common misconception. The Singleton design pattern is probably the worst named of all of the design patterns, because according to the GoF book the pattern constrains the number of instances created. Note that it does NOT say there's only one instance. For example, a class used to talk to a server could be designed as a Singleton. If we know there's exactly two such servers we can talk to then exposing a ServerA and a ServerB property means we're still following the Singleton pattern even though we create an instance of the class for both of those. I know, not the best example of this, but strictly speaking this is what Singleton does. It restricts the number of instances, usually but not exclusively to one.

    You state that you can't rely on static being thread-safe, and said you'd provide documentation for this, but I don't see any links. I really want to see said documentation, because the CLR specification says otherwise (https://stackoverflow.com/questions/7095/is-the-c-sharp-static-constructor-thread-safe). The BCL uses this all over the place as well (one example https://referencesource.microsoft.com/#mscorlib/system/stringcomparer.cs). Really, by far the best implementation of Singleton in .NET should use static initialization/construction not only because it's simpler, but because it is thread-safe.

    In your implementation you use the DCL. I firmly believe you should never do this. The nuances for getting it right (in many languages you can't) are complicated. In the video several times you admit to not fully understanding it, and make some declarations about this that are not entirely accurate (such as stating volatile will cause it to wait until the object is fully constructed... this statement shows you understand the issue exists, but not what that issue is exactly, or how this fix works). So, since there's really no reason to use this construct, it's better not to. In this case, static construction/initialization would deal with this for you. Don't agree that it does? OK, use Lazy<T> instead then. Don't rely on a very low level construct you don't fully understand here, use a higher level construct you can reason about instead.

    You threw in IDisposable in this discussion for some reason, and really messed it up. Firstly, the whole point of a Singleton is that it will exist for the entire lifetime of the application, so there's exactly zero need to make it IDisposable. Who's going to call Dispose and when? The Main method before exit? What purpose does that serve? You probably made this mistake because it appears you don't understand the difference between IDisposable and finalizers. You seemed to indicate in the video that the GC will call Dispose. It won't. That's why your finalizer calls it. You did follow the "IDisposable pattern" correctly, but frankly that pattern is broken and wrong (explaining that statement would take too long here, but the reality is you probably should never code a finalizer yourself). However, your implementation for Dispose is pointless at best. Setting members to null in Dispose is in all likelihood a waste of time and effort. Theoretically this could allow an object to be collected sooner, but in reality this will hardly ever be true, and unless that object tracks unmanaged resources there's no benefit to it happening sooner anyway. No, Dispose is needed only if you have members that need to be disposed (or, ignoring my comment about the "IDisposable pattern" being broken, to release unmanaged resources). Bottom line, including IDisposable here was wrong, will confuse far too many people, and you didn't do it correctly in any event.

  • User profile image
    japikse

    @wkempf: Thank you for your comment.  The beautiful thing about software architecture is that there are wide and varied opinions, and as I'm pretty sure that I've said in each of these videos (most assuredly in the opener), that the importance is the pattern and not necessarily the implementation.  I will address some of your points, according to my opinion and my opinion alone. That being said, I refuse to get drawn into a battle of opinion online, let alone on someone else's property.  

    I believe I stated (at least I always try to make a point of saying) in regards to the singleton that 1) it's the most over used pattern and 2) it's often used incorrectly.  I also stated that in .NET, singleton usually means one instance per app domain, and not simply one instance in the entire world.

    As far as thread safety of static classes, while I have a lot of respect for Stack Overflow, I trust the MS documentation more.  I researched this heavily before the show, because I've know for quite some time (from the C# language team) that there were instances where static was not thread safe, but I wasn't going to record with "someone once told me" as my explanation.  From this article (from P&P) https://msdn.microsoft.com/en-us/library/ff650316.aspx?f=255&MSPPError=-2147217396:

    Static initialization is suitable for most situations. When your application must delay the instantiation, use a non-default constructor or perform other tasks before the instantiation, and work in a multithreaded environment, you need a different solution. Cases do exist, however, in which you cannot rely on the common language runtime to ensure thread safety, as in the Static Initialization example. In such cases, you must use specific language capabilities to ensure that only one instance of the object is created in the presence of multiple threads. One of the more common solutions is to use the Double-Check Locking [Lea99] idiom to keep separate threads from creating new instances of the singleton at the same time.

    From the same article from P&P:

    Also, the variable is declared to be volatile to ensure that assignment to the instance variable completes before the instance variable can be accessed. Lastly, this approach uses a syncRoot instance to lock on, rather than locking on the type itself, to avoid deadlocks.

    As for IDisposable, while I agree it wasn't the best place to squeeze it in, I don't agree that this is a horrible place for this.  Regardless of the lifetime on an object, as some point is needs to release its resources.  Do you want to wait for tree passes of GC, or have control over it?  I know for a fact a large community run conference (1600+ people) that were using NHibernate for the DAL, and created a new context on every request.  This conference typically sells out in a matter of hours, and the load crushed the site.  They wrapped the creation of the context in a singleton, and were able to complete registration.  This class doesn't need to exist for the entire lifetime of the site, it just needs to guarantee that there is only one.  So should I call that out more? Probably.  Should I move it to another part of the talk?  Probably true as well.

    I don't recall my exact wording I used, so if I misspoke about which gets called by the GC, dispose, or the finalizer, then I apologize.  A lot can happen on one take recording. As for your comment that I did it incorrectly, I don't believe that I did it incorrectly, at least not according to P&P, but if you do, than I respect your opinion, and I simply agree to disagree.

    Thank you for your comment.

  • User profile image
    wkempf

    I wasn't suggesting you trust StackOverflow... I was being lazy. The answers there referenced the specification, which is what I should have hunted down and linked to. As is always appropriate, you should trust the spec before any other source, which includes P&P. Especially when that page is so vague. I could be wrong, but what I believe the page is truly pointing out is that determining the order in which initialization occurs isn't always possible, much less easy to do. So, if your initialization depends on other static members being initialized, yes there can be race conditions or even other factors that cause you problems here. That's not the case in most Singleton classes, however.

    I'll grant you that avoiding the DCL is an opinion... but one that's founded on a great deal of knowledge of this space. That P&P page that states "The common language runtime resolves issues related to using Double-Check Locking that are common in other environments." is correct, but remarkably misleading. The common language runtime provides enough guarantees and features to enable you to write the pattern correctly, but it's still a terribly complicated topic that most people don't understand and can easily code incorrectly. You've done so correctly here, and given good comments about what's needed, but in the video you demonstrate a lack of actually understanding it. Just my opinion, but when there's alternatives to such dangerous patterns we're better suited to use them instead, and most certainly should be teaching them.

    On IDisposable, I ask again, when would you call Dispose? The only logical time you could do this would be at application shutdown, which is pointless. The act of shutting the application down will already clean things up. There's no point in explicitly doing so. Your example with NHibernate seems incomplete. When did they call Dispose in that example? I'm willing to bet they didn't.

    I'd love to hear where P&P suggests you set a variable to null in a Dispose method. It's a pointless thing to do. If there were a reason to do this then EVERY class should implement IDisposable and we'd be writing a lot of boilerplate code to null out our members and wrapping absolutely everything in using statements. If you follow the "IDisposable pattern" you only implement IDisposable if your type contains other IDisposable types or directly uses unmanaged resources. Your Singleton doesn't, and should NOT be IDisposable, and far more importantly should not have a finalizer. There's significant cost to adding a finalizer to a class, and given we have safe handles (and the pattern we can follow for other resources) I believe the "IDisposable pattern" is broken and we should never (unless implementing a safe handle) implement a finalizer. That bit is opinion, but based on technical reasoning. Don't let that tangent derail your thinking here though, as it doesn't really have any bearing on the fact that you have no cause to use IDisposable here.

  • User profile image
    Dev_Raj

    Singleton pattern example (in Net framework) : SqlClientFactory class

  • User profile image
    tgrt

    @wkempf: @japikse:

    That's not true. Although I'd argue that most Singletons are going to live for the lifetime of the AppDomain, it's possible that you may want to clean up before hand. Additionally, your Singleton may use system resources (especially unmanaged resources) that you want to be explicit about cleaning up. After all, there are no guarantees made that finalization (invocation of the destructor) will be invoked when the AppDomain is shutdown (more info).

    I disagree with the assertion that you should implement IDisposable in what was seemingly inferred as all the time. IDisposable is a design pattern, not a classic pattern, but it's still a pattern. It should be used when the situation warrants it. Furthermore, a destructor should never be added unless absolutely necessary (see 1.6.7.6 in the language specification).

  • User profile image
    tgrt

    @japikse:

    One other point I wanted to make on the video is regarding sealed classes. In the video you indicate that a sealed class is a code smell. I strongly disagree with that sentiment as do others. For example, Jeffrey Richter wrote about it in his book CLR via C# when he said it should be the default for new classes.

    There are run-time optimizations that are available for sealed classes according to the language specification 10.1.1.2; and more specifically with versioning a sealed class can be changed to unsealed later and not break compatibility and methods in a sealed concrete class can be called non-virtually.

  • User profile image
    SirGru

    Thanks for the talk.
    For more information about the complexity involved in singletons, I could recommend to your viewers to read this: http://csharpindepth.com/Articles/General/Singleton.aspx . It discusses the various nuances brought up in this talk.
    I firmly believe we should get to the bottom of the question if there can be race conditions in a specific case of a static singleton, as seen in the Fourth example from the link above. Maybe do an updated talk which would involve that and not talk about IDisposable because that is an entirely different can of worms that should be a separate pattern / talk.
    Also, we need a resource that describes a singleton created using inheritance. There is way too much boilerplate code for a simple singleton and it is possible to create it using generics and inheritance.

  • User profile image
    jakubsuchyb​io

    @japikse:

    Sorry, but I am still lost about that thread-safety...

    P&P resource for Singleton is flagged as "Retired Content" with "This content is outdated and is no longer being maintained."

    After a little research I haven't found any other material from new docs or msdn on this topic.

    "Cases do exist, however, in which you cannot rely on the common language runtime to ensure thread safety, as in the Static Initialization example."

    I haven't found any information/reproduction in which static initialization isn't thread-safe.

    If there do exist some case. Could you provide some resource or repro for it?

    Until then I am convinced by other sources that static initialization is indeed thread-safe.

    Thx

  • User profile image
    JabberMyHutt

    Doesn't providing global access to mutable shared state or side-effects remove all semblance of modular reasoning, and hence significantly increase the complexity of your code?

    More over, I'd context design patterns being a "best practice" - instead I'd say they just name a common style of solving a problem. It's useful to name patterns even if they are bad so that we can easily discuss them.

  • User profile image
    stevetaylor​uk

    To quote Jon Skeet:

    What makes you say that either a singleton or a static method isn't thread-safe? Usually both should be implemented to be thread-safe.

    The big difference between a singleton and a bunch of static methods is that singletons can implement interfaces (or derive from useful base classes, although that's less common, in my experience), so you can pass around the singleton as if it were "just another" implementation.

    SO Reference

  • User profile image
    OFS

    @wkempf: Could not agree more about your comment on IDisposable. Even if you set aside the fact that it is a singleton there was no reason to implement IDisposable. Sadly this is a very common mistake and once devs see IDisposable used like this it is hard to break them of the habit. If I had a nickel for every time I saw a class that implements IDisposable to only set managed variables to null I'd be rich.

  • User profile image
    Artemious

    Keep things simple. Stick to the topic. This is overcomplicated with IDisposable and thread-safety.

    Although, I learned many new things, this is not what I came here for.

Add Your 2 Cents