Singleton Class in multithreading app, locking suggestion
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++;
}
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)
{
// ...
}
}
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
}
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();
}
}
This is fairly common. Locking/unlocking in the getters/setters is much safer, (you cannot forget to do it), and more convenient, (the lock does not have to be directly accessable everywhere you use the property), than an external lock on every property access.
Rgds, Martin
精彩评论