Singleton Class in multithreading app, locking suggestion

Question

I have singleton class which is shared in several threads. To prevent multiple access issues I use Lock method when accessing one or another property of the class. The question would be is it possible to improve code and put Lock method inside singleton class rather than putting it every time when the class property is accessed in code?

/* Class code*/
   public class ServerStatus
    {


        private static ServerStatus _instance;
        public static ServerStatus Instance
        {
            get { return _instance ?? (_instance = new ServerStatus()); }
            set { _instance = value; }
        }

        ServerStatus()
        {
            PistonCount = 0;
            PistonQueue = new List<string>();
            ErrorList = new List<string>();
        }




        public int PistonCount { get; set; }

        public List<string> PistonQueue { get; set; }

        public List<string> ErrorList { get; set; }
    }



 /*Code for accessing class properties*/
private static readonly object Locker = new object();    
/*Skip*/

lock (Locker)
{
 ServerStatus.Instance.PistonQueue.Add(e.FullPath);
}
    /*Skip*/

lock (Locker)
{
    ServerStatus.Instance.PistonCount++;
}

Answer 1

ServerStatus should maintain its own synchronization, not external clients of this class. That being said, you'll need to refactor ServerStatus and create a few thread-safe (with locking) methods:

Remove these properties: public List<string> PistonQueue { get; set; } since even though you can lock inside these properties, you can't control what clients do once they get a hold of the actual PistonQueue.

...and replace with methods such as (sorry pseudo-code, I can't be bothered to think today):

public PistonQueueAdd(string fullPath)
{
    lock(_serverStatusSyncRoot)
    {
        // ...
    }
}

Answer 2

This is the singleton thread-safe pattern I use in case you are interested:

    public class DataAccess
{
    #region Singleton

    private static readonly object m_SyncRoot = new Object();

    private static volatile DataAccess m_SingleInstance;

    public static DataAccess Instance
    {
        get
        {
            if (m_SingleInstance == null)
            {
                lock (m_SyncRoot)
                {
                    if (m_SingleInstance == null)
                        m_SingleInstance = new DataAccess();
                }
            }

            return m_SingleInstance;
        }
    }

    private DataAccess()
    {
    }

    #endregion
}

Answer 3

IMHO, this is the definitive solution for thread-safe locking in a singleton. From it (fifth on the list):

public sealed class Singleton
{
    private Singleton()
    {
    }

    public static Singleton Instance { get { return Nested.instance; } }

    private class Nested
    {
        // Explicit static constructor to tell C# compiler
        // not to mark type as beforefieldinit
        static Nested()
        {
        }

        internal static readonly Singleton instance = new Singleton();
    }
}