CODEWITHSUNDEEP
c#.netconcurrencylocking
Jens
Jens

Reputation: 25

How to properly lock a collection

I've been trying to wrap my head around locks but I can't seem to figure it out. The code below uses a lock but still gives a 'collection was modified' error. What am I missing?


    class Program
    {
        static List<int> lEntries = new List<int>();
        static readonly object entriesLock = new object();

        public static List<int> Entries {
            get { lock (entriesLock) { return lEntries; } }
            set { lock (entriesLock) { lEntries = value; } }
        }

        static void Main(string[] args)
        {
            // Run 20 times to reproduce the issue more often
            for (int i = 0; i < 20; i++)
            {
                Task.Run(() =>
                {
                    for (int j = 0; j < 10000; j++)
                    {
                        Entries.Add(j);
                    }
                });

                Task.Run(() =>
                {
                    for (int i = 0; i < 1000; i++)
                    {
                        Entries.Average(); // System.InvalidOperationException: 'Collection was modified; enumeration operation may not execute.'
                    }
                });
            }

            Console.ReadLine();
        }
    }

Upvotes: 1

Views: 789

Answers (1)

Jack T. Spades
Jack T. Spades

Reputation: 1006

Locks only last for their scope.

lock (entriesLock)
{
  //safe to access here.
}
// no longer safe

As such, your attempt of returning a locked list is unfortunately meaningless, as the lock expires right away when the getter/setter is left. Use the lock outside when you actually access the list.

for (int j = 0; j < 10000; j++)
{
  lock (entriesLock)
  {
    lEntries.Add(j);
  }
}

// or

lock (entriesLock)
{
  for (int j = 0; j < 10000; j++)
  {
    lEntries.Add(j);
  }
}

Upvotes: 5

Related Questions