Background information for "possible incorrect implementation of double checked locking"?

Hi -

R# 6.0 has two new code inspections for double checked locking:

  • One suggests using a volatile variable, which is obvious and reasonable; however, is it really necessary?
  • The other says "... Read access to checked field", which I do not understand.

Is there any background information on these suggestions - somewhere on the web (publication, ...) or elsewhere? I googled the terms in various combinations, but did not find any.

Thx a lot!
Harald M.

4 comments

There's nothing obvious in the Code Inspection Wiki (which isn't really a wiki!).

Edit: The Code Inspection Wiki has been updated:


Can you past an example of the code which produces these warnings?

0

How to produce the two warnings? Simple:

-------------

namespace Whatevers {

    class Whatever {

        private static readonly object _lock = new object();

        private static Whatever _instance = null;

        public static Whatever Instance {

            get {

                if (_instance == null) {

                    lock (_lock) {

                        if (_instance == null) {

                            _instance = new Whatever(); // This yields "... must be volatile ..." --> can be removed by making _instance volatile as suggested.

                            _instance.Init(); // This yields "...Read access to checked field".

                        }

                    }

                }

                return _instance;

            }

        }

        private void Init() {

            // empty

        }

    }

}

---------------

Regards
Harald :M.

0

There are several references which explain the problems with this approach:
http://msdn.microsoft.com/en-gb/magazine/cc163715.aspx#S10
http://www.bluebytesoftware.com/blog/PermaLink,guid,543d89ad-8d57-4a51-b7c9-a821e3992bf6.aspx
http://en.wikipedia.org/wiki/Double-checked_locking

It basically seems to boil down to the memory model not guaranteeing the order of reads and writes without an explicit memory barrier or volatile semantics.

If possible, you should move the contents of the "Init()" method to a private instance constructor. You could then replace your code with:

private static readonly Whatever _instance = new Whatever();

public static Whatever Instance
{
    get { return _instance; }
}

private Whatever()
{
    // Init logic here...
}


If you can't do that for some reason, then try changing your code to:

private static readonly object _lock = new object();
private static volatile Whatever _instance;

public static Whatever Instance
{
    get
    {
        if (_instance == null)
        {
            lock (_lock)
            {
                if (_instance == null)
                {
                    var temp = new Whatever();
                    temp.Init();
                    _instance = temp;
                }
            }
        }
        
        return _instance;
    }
}

0

Further info about the singleton pattern which might be worth a read (also covers laziness as well as .Net 4's Lazy<T> type) by Jon Skeet: http://csharpindepth.com/Articles/General/Singleton.aspx

0

Please sign in to leave a comment.