0

I'm writing a C# library that needs to treat a List at high speed via multiple Timers. I ran into very erratic error, where I try to remove an element that I know for sure is contained into the List but the program returns the following error :

System.IndexOutOfRangeException : 'index was outside the bounds of the array.'

I've made a simple example to reproduce this behaviour. Because of that issue's randomness, I've pushed hard on List operations so it throws the error right away. So this example is necessary "weird". I've made a public repo on here : Issue Example Repo

Basically, here's what I'm dealing with:

        list = new List<DummyElement>();
        for (int i = 0; i < 1000; i++)
        {
            Timer addTimer = new Timer(0.01f);
            addTimer.Start();
            addTimer.Elapsed += AddItem;
            Timer removeTimer = new Timer(0.01f);
            removeTimer.Start();
            removeTimer.Elapsed += RemoveItem;
        }
        void AddItem(object source, ElapsedEventArgs e)
        {
            list.Add(new DummyElement());
        }

        void RemoveItem(object source, ElapsedEventArgs e)
        {
            int listCount = list.Count;
            if (listCount > 0)                   // This condition is successfully passed, so there is at least one element on the list
            {
                list.RemoveAt(0);            // This line throw an IndexOutOfRangeException error
            }
        }

I believe it is a thread related issue, as if the list count was changing AFTER the condition was successfully passed.

I know nothing about thread, how can I deal with this issue?

Community
  • 1
  • 1
LePioo
  • 137
  • 1
  • 11
  • What kind of `Timer` are you using? [`System.Windows.Forms.Timer`](https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.timer)? [`System.Timers.Timer`](https://learn.microsoft.com/en-us/dotnet/api/system.timers.timer)? [`System.Threading.Timer`](https://learn.microsoft.com/en-us/dotnet/api/system.threading.timer)? – Theodor Zoulias Feb 12 '20 at 12:55
  • I use System.Timers.Timer – LePioo Feb 12 '20 at 19:13
  • With `System.Windows.Forms.Timer` you wouldn't have to worry about thread-safety issues, because it's `Tick` event runs in a single thread (the UI thread). – Theodor Zoulias Feb 13 '20 at 05:42
  • I can't find System.Windows.Forms.Timer on my project, is it not dedicated to Windows Form Application? – LePioo Feb 13 '20 at 18:37
  • Yes, it is a Windows Forms specific component. – Theodor Zoulias Feb 13 '20 at 18:39

1 Answers1

2

In the For loop that goes upto 1000 - you are creating about 1000 Timers that add an item into list and 1000 timers that remove first item.

Since you haven't used any synchronization here's what happens:- Say there is 1 item in List and 2 RemoveItems are executing. both see listCount > 0 as True then one of them goes ahead and removes the Item at 0th Index while the other gets an Exception cause there is no item to remove now.

Now I cant suggest a solution to this by just looking at the code. I also need to understand the intent.

This is a Text Book Producer Consumer problem so the Text book advice here is using Lock construct:

Assume you have a class member like:

private object _lockMe = new object();

void RemoveItem(object source, ElapsedEventArgs e)
{
    lock(_lockMe)
    {
        int listCount = list.Count;
        if (listCount > 0)                   // This condition is successfully passed, so there is at least one element on the list
        {
            list.RemoveAt(0);            // This line throw an IndexOutOfRangeException error
        }
     }
}
Prateek Shrivastava
  • 1,877
  • 1
  • 10
  • 17
  • Don't `lock` on the `list` itself. That's a bad idea. – Enigmativity Feb 12 '20 at 00:13
  • I was just reading about the lock statement, it looked really complicated to use, you've made it simple ! It solves the problem on this reproducing example, I'm gonna try to apply this into my library! Thanks a lot! – LePioo Feb 12 '20 at 00:14
  • @Enigmativity I don't really understand what this parameter is here for? – LePioo Feb 12 '20 at 00:15
  • @LePioo - The parameter in the `lock` statement? – Enigmativity Feb 12 '20 at 00:17
  • You can create any reference type object for locking purpose. Let me edit the example. – Prateek Shrivastava Feb 12 '20 at 00:20
  • I'm quite confused about the object you put as parameter into the lock statement... Is it any class member object? – LePioo Feb 12 '20 at 00:34
  • 3
    @LePioo - It just needs to be any reference object that you hold a reference to - preferably one that is private field in the class. – Enigmativity Feb 12 '20 at 00:38
  • Thx a lot guys, I'll look into that – LePioo Feb 12 '20 at 00:41
  • @Enigmativity it worked as long as I use only one instance of the class at the same time. However when the locker object is set to static it works like a charm! – LePioo Feb 12 '20 at 00:48
  • Making the lock object static would mean it will be shared across all instances of your class. That seems wrong to me as timer operating on local list of one instance will block other removeitem timers of all other instances. As I said, I dont know what you are trying to achieve but this is one way of doing things. – Prateek Shrivastava Feb 12 '20 at 01:01
  • @LePioo - I'm not sure what you mean by "use only one instance ... at the same time" and "set to static". Both of those don't seem to be what you should be doing. Can you let us know the code that you've used? – Enigmativity Feb 12 '20 at 01:12
  • It's hard to not be evasive because my project is dozen of files and classes, but mainly I have to make a Timer wrapper class that will be instanciated many times and treat list at different speed rate. When I instantiate multiple times this wrapper I have to use 'private static object _lockMe = new object();' or the overlapping behaviour will happen again. – LePioo Feb 12 '20 at 01:21
  • @LePioo - If the List will always be Singleton in your library/project - then using static lock object wont matter much (IMO). However, if there can be multiple instances of this class where same behavior is expected. (i.e. Every object of the class will have one List and all the different producer consumer timers) Then you cannot use Static lock object. However, you know your design/objective way better than me. – Prateek Shrivastava Feb 12 '20 at 02:59
  • @PrateekShrivastava the list is outside the class – LePioo Feb 12 '20 at 04:42