0

I have a singleton class where a list of objects is placed. So I have a class that sets items to that list. Then, I have another class that gets the list of objects from that singleton class. My issue is when I receive an nservicebus message and get the list from the singleton class, there are times that the list does not contain any objects. And there are times that the objects exist. So, what I did is every time I get the singleton instance I execute 'GetHashCode' and confirmed that there are 2 different instances of the Singleton class. What did I implement incorrectly with my code?

public class SingletonClass
{
    private static readonly object _lockObj = new object();
    private static readonly object _lockObjList = new object();

    static SingletonClass _singletonClass;

    private static List<object> _objList;


    private SingletonClass()
    {

    }

    public static SingletonClass Instance
    {
        get
        {
            lock(_lockObj)
            {
                if (null == _singletonClass)
                {
                    _singletonClass= new SingletonClass();
                    _objList = new List<object>();
                }
                return _singletonClass;
            }
        }
    }

    public List<obj> GetList()
    {
        lock(_lockObjList)
        {
            return _objList;
        }
    }

    public void UpdateProgress(int index, double value)
    {
        lock(_lockObjList)
        {
            _objList[index].Progress = value;
        }
    }


    public void SetList(List<obj> objs)
    {
        lock(_lockObjList)
        {
            _objList = objs;
        }
    }
}
public class MessageHandler : HubInvoker<MessageHub>
{

    public MessageHandler () {}

    public void OnReceiveMessage(object sender, MessageArgs args)
    {
        var list = SingletonClass.Instance.GetList();
        if(list != null){
            var i = 0;
            for(; i < list.Length && list[i].Id == args.Id; i++);

            if(i < list.Length)
            {
                SingletonClass.Instance.UpdateProgress(i, args.Progress);
            }
        }
    }
}
public class ObjController
{
     public ObjController() {}

     public void SetList(List<obj> objs)
     {
         SingletonClass.Instance.SetList(objs);
     }
}

EDITED

I've added some codes above for more information of my implementation.

Simon
  • 33,714
  • 21
  • 133
  • 202
chary
  • 57
  • 13
  • I would probably just go for a `Lazy` approach. Don't make the class a singleton class, just store it's value in a `static Lazy` field – Callum Linington Feb 09 '17 at 11:53
  • Why do you need this List to be static singleton in the first place anyway? – Callum Linington Feb 09 '17 at 11:55
  • @CallumLinington The list is modified in the application asynchronously. Multiple classes access and modify the list – chary Feb 09 '17 at 11:58
  • I tried the Lazy approach, still having 2 instances – chary Feb 09 '17 at 12:06
  • How are you accessing this list, do you need to find items by key, or just process what ever is in there? – Callum Linington Feb 09 '17 at 12:44
  • @CallumLinington I'm updating a property in that list. I'm doing a progress % update. And the list contains the items that are in progress. So each item in that list has a %-age property which I update when I receive an event from the NServiceBus. The update of the %-age property also happens in the SingletonClass – chary Feb 10 '17 at 02:31
  • So, the correct way to implement this would be to have a concurrent bag, you could do it with a concurrent dictionary! That will remove that locking code and allow true multithreaded code. Then we will have to look at getting this singleton working – Callum Linington Feb 10 '17 at 07:43
  • @CallumLinington Yup, I've already tried the Concurrent Dictionary or ConcurrentQueue, Still I get empty bag – chary Feb 13 '17 at 01:43

1 Answers1

0

You will need to implement double checked locking, see here and depending on your setup a volatile keyword as well. See below code:

public static SingletonClass Instance
{
    get
    {
        if (null == _singletonClass)
        {
            lock (_lockObj)
            {
                if (null == _singletonClass)
                {
                    _singletonClass = new SingletonClass();
                    _objList = new List<object>();
                }
                return _singletonClass;
            }
        }
    }
}
Community
  • 1
  • 1
Jeroen
  • 3,443
  • 2
  • 13
  • 15