7

What is the benefit of or reason for using this pattern?..

public sealed class myStaticClass
{
    private static bool _initialized;
    private static object _lockObject;

    private static string _someStaticField;
    private static int _anotherStaticField;
    private static string _nthStaticField;

    static myStaticClass()
    {
        _initialized = false;  
        _lockObject = new object();
    }

    public myStaticClass()
    {
    }

    public static void Initialize()
    {
        if(!_initialized)
        {
            lock(_lockObject)
            {
                if(!_initialized)
                {
                    //do initializing
                    _someStaticField = someApplicationSetting;
                    _anotherStaticField = anotherApplicationSetting;
                    _nthStaticField = nthApplicationSetting;

                    _initialized = true;
                }
            }
        }
    }

    public static string NthStaticField 
    { 
        get {

            Initialize();
            return _nthOtherField;
        }
    }
}

If a static constructor is guarenteed to be called before any of the class members are ever accessed and it is only ever called once then why not just put all of the initialization logic in the static constructor?

EDIT: I have updated the pattern to better reflect the class in the .net framework where I have discovered it. I have change the static modifier on the class to sealed and I have also added an empty public constructor.

ps. Incase you would like to know, the class where I have seen this pattern is the FormsAuthentication class.

Duncan Gravill
  • 4,552
  • 7
  • 34
  • 51
  • Look up "double checked locking" and/or "lazy initialization" for C#. There is a nice article linked to on several posts that talks about the different ways to get this sort of lazy [one-time-when-needed] initialization. (And the code posted is actually *not* one of the recommend ways in many cases ;-) –  Mar 16 '12 at 22:33
  • Ahh, here that famous article from C# In Depth: http://csharpindepth.com/Articles/General/Singleton.aspx There are 5 approaches listed, with explanations. –  Mar 16 '12 at 22:39
  • http://stackoverflow.com/questions/6011068/name-for-this-pattern-answer-lazy-initialization-with-double-checked-locking , http://stackoverflow.com/questions/978759/what-is-lazy-initialization-c-net –  Mar 16 '12 at 22:44
  • 2
    @pst is right but should have been more forceful. **This code is deeply broken and wrong.** You cannot do double-checked locking safely like this! The fundamental rule of double checked locking is that *the thing you **test** has to be the **only** thing you mutate*. You can't use a flag to control double-checked locking and then *mutate some completely other variable*; that's in no way guaranteed to be safe. If you do not fully understand the memory model of the runtime you should not be attempting to write low-lock code. – Eric Lippert Mar 16 '12 at 23:03
  • 1
    Read my answer in the first link that pst posted there; this is yet another example of the famous "dangerously broken incorrectly implemented double checked locking pattern". – Eric Lippert Mar 16 '12 at 23:05
  • pst & Erik, thank you for the knowledge and links. I am aware of the basic principal of locks but don't think I have ever actually used them so I shall have to spend some time reading through and taking this in. I am supprised to hear that this is a **deeply broken and wrong** pattern as I think I have actually seen this in use in the .net framework. I believe however that the only place the `_initialized` field is actually set is from within the `Initialize` method. If that is the case is it still broken or is it just dangerous? – Duncan Gravill Mar 16 '12 at 23:51
  • @FunkyFresh84 It seems you still don't understand what the problem with double checked locking is. And yes, it's still broken. Look at the links from others for a detailed explanation. BTW, what would be the difference between “broken” and “dangerous”? If by “dangerous” you mean something like “the code works most of the time, but will not work always”, then I would say that that code is broken. – svick Mar 17 '12 at 18:19

3 Answers3

2

The only reason to do so would be if initialization is expensive (in terms of CPU/RAM) OR something needed is only available later during runtime IMO... this implementation delays initialization until the latest possible moment (right before first access). It is kind of what Lazy<T> offers...

Yahia
  • 69,653
  • 9
  • 115
  • 144
1

Possibly, the ApplicationSettings are not available until later on during run-time.

torrential coding
  • 1,755
  • 2
  • 24
  • 34
0

Ok I have now read and considered the links given and have come to the conclusion that this particular example is not broken.

I shall explain why...

Firstly it is not the same pattern as the one in csharpindepth article csharpindepth.com/Articles/General/Singleton.aspx. The pattern in this article is for a singleton. The pattern I have presented is not a singleton, it has a public constructor, however all of it's fields are static. So the big difference is that the csharpindepth pattern returns a reference to a singleton object even though it may not have actually been fully instantiated. The key point is that the instance may be assigned to the reference before the constructor has completed.

public sealed class Singleton
{
 private static Singleton instance = null;
 private static readonly object padlock = new object();

Singleton()
 {
 }

public static Singleton Instance
 {
     get
     {
         if (instance == null)
         {
             lock (padlock)
             {
                 if (instance == null)
                 {
                     instance = new Singleton();
                 }
             }
         }
         return instance;
     }
 }
} 

this is the csharpindepth pattern and it works like this..

If there are 3 threads A, B and C. Thread A calls the Instance method, instance is null, it gets the lock. Now before thread A creates a Sigleton instance along comes thread B, instance is still null. But A has the lock so B waits. While B is waiting thread A executes the code instance = new Instance(). At this point instance is set to an instance of Singleton however it is possible that it has not actually been constructed yet. This is where the thread safety breaks. Along comes thread C, it sees that instance is not null so the Instance method returns a referance to instance. But thread A has not yet finished constructing the instance so Thread C and any other threads that entered at the same time as thread C get a reference to a dud Singleton instance. Finally A finishes, B gets the lock but the work is now done so it exits.

The pattern I have presented is significantly different. The field that is being used for the double check is not an instance that is created in the Initialize method. It is a simple private static bool that is used as a switch. Since it is not a reference to a Singleton object that is yet to be constructed and since it is never accessed outside of this class there is not issue.

There would be an issue if the private static bool _initialized switch was set to true at the top before the class' fields had been initialized but it isn't. It is the last thing to happen before the code gives up the lock. So there is no chance of a thread C reading _initialized and mistaking the class as having been initialized. And so it is not in anyway boken.

Duncan Gravill
  • 4,552
  • 7
  • 34
  • 51