CODEWITHSUNDEEP
c#listasynchronousconcurrencysynchronization
buciz
buciz

Reputation: 73

SynchronizedCollection InvalidOperationException/System.ArgumentException

I wrote some classes to test multithreading using SynchronizedCollection.

class MultithreadTesting
{
    public readonly SynchronizedCollection<int> testlist = new SynchronizedCollection<int>();
    public SynchronizedReadOnlyCollection<int> pubReadOnlyProperty
    {
        get
        {
            return new SynchronizedReadOnlyCollection<int>(testlist.SyncRoot, testlist);
        }
    }

    public void Test()
    {
        int numthreads = 20;
        Thread[] threads = new Thread[numthreads];
        List<Task> taskList = new List<Task>();
        for (int i = 0; i < numthreads / 2; i++)
        {
            taskList.Add(Task.Factory.StartNew(() =>
            {
                for (int j = 0; j < 100000; j++)
                {
                    testlist.Add(42);
                }
            }));
        }

        for (int i = numthreads / 2; i < numthreads; i++)
        {
            taskList.Add(Task.Factory.StartNew(() =>
            {
                var sum = 0;
                foreach (int num in pubReadOnlyProperty)
                {
                    sum += num;
                }
            }));
        }
        Task.WaitAll(taskList.ToArray());
        testlist.Clear();
    }
}

to run it I use

    MultithreadTesting test = new MultithreadTesting();
    while (true)
        test.Test();

But the code throws me System.ArgumentException: 'Destination array was not long enough. Check destIndex and length, and the array's lower bounds.'

If I try to use testlist in foreach, I get

System.InvalidOperationException: 'Collection was modified; enumeration operation may not execute.'

However, MSDN tells us

SynchronizedReadOnlyCollection Class

Provides a thread-safe, read-only collection that contains objects of a type specified by the generic parameter as elements.

Upvotes: 3

Views: 345

Answers (1)

Risto M
Risto M

Reputation: 3009

The root cause of the error is that List<T> construction is not thread-safe.

Let's see what happens when constructing new SynchronizedReadOnlyCollection. Exception occurs in following line:

return new SynchronizedReadOnlyCollection<int>(testlist.SyncRoot, testlist);

As exception StackTrace tells us, there is List<T>..ctor involved in construction process:

at System.Collections.Generic.SynchronizedCollection`1.CopyTo(T[] array, Int32 index)
at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
at System.Collections.Generic.SynchronizedReadOnlyCollection`1..ctor(Object syncRoot, IEnumerable`1 list)

Following snippet from List<T> constructor shows where error happens. Code is copied from MS reference source, I cleaned unnecessary parts of code for easier reading. Please notice that between comments (1) and (2) there are other threads manipulating collection:

public List(IEnumerable<T> collection) {
    ICollection<T> c = collection as ICollection<T>;
    // (1) count is now current Count of collection
    int count = c.Count;
    // other threads can modify collection meanwhile
    if (count == 0)
    {
        _items = _emptyArray;
    }
    else {
        _items = new T[count];
        // (2) SynchronizedCollection.CopyTo is called (which itself is thread-safe)
        // Collection can still be modified between (1) and (2) 
        // No when _items.Count != c.Count -> Exception is raised.
        c.CopyTo(_items, 0);
        _size = count;
    }
}

Solution

The problem can easily be fixed with locking testlist modification while constructing new SynchronizedReadOnlyCollection. 

public SynchronizedReadOnlyCollection<int> pubReadOnlyProperty
{
    get
    {
        lock (testlist.SyncRoot)
        {
            return new SynchronizedReadOnlyCollection<int>(testlist.SyncRoot, testlist);
        }
    }
}

Upvotes: 2

Related Questions