Collection was modified; enumeration operation may not execute during select

1
votes

Unhandled exception has occurred. System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
at System.Collections.Generic.List`1.Enumerator.MoveNext()
at System.Linq.Enumerable.WhereSelectListIterator`2.MoveNext()
at System.Linq.Enumerable.Sum(IEnumerable`1 source)
at Check.get_TotalChange()
at CheckChanged(ICheck check)

If I run my code like 5 thousand times per day like 2 times it will result in this error and the program will crash.. What's wrong with my code? I followed the exception information and I think this is the code that may cause the exception. Anyone have an idea what kind of problem that is?

public decimal TotalChange
{
    get
    {
        var change = (from IPayment payment in _payments where payment.Amount < 0 select payment.Amount).Sum();
        var paid = (from IPayment payment in _payments select payment.Amount).Sum();
        return paid <= Total && Total > 0 ? Math.Abs(change) : paid - Total;
    }
}

I changed it up to:

    private readonly object _totalChangeLock = new object();
    public decimal TotalChange
    {
        get
        {
            lock (_totalChangeLock)
            {
                var change =
                    (from IPayment payment in _payments where payment.Amount < 0 select payment.Amount).Sum();
                var paid = (from IPayment payment in _payments select payment.Amount).Sum();
                return paid <= Total ? Math.Abs(change) : paid - Total;
            }
        }
    }

Also I do have 2 methods that modify _payments. I locked them too. Hope that doesn't cause any issues.

    public void AddPayment(IPayment payment)
    {
        lock (_paymentsLock)
        {
            if (_payments.Contains(payment)) return;
            _payments.Add(payment);
        }
    }

    public void RemovePayment(IPayment payment)
    {
        lock (_paymentsLock)
        {
            if (_payments.Contains(payment))
                _payments.Remove(payment);
        }
    }
2
c#.netlinq
Assumption: you are doing things in a multithreaded way. The collections change while you're enumerating them, as the error says, since you're not using any locking mechanisms. – Sami Kuhmonen
@Wkjjjj is this method called in a loop – Maverick
You are right, I'm using multithreads and multiple threads can call to this method from different places at the same time. So I just need to lock _payments I guess? – Wkjjjj
@Wkjjjj Exactly. just like dlxeon did in his answer. – Maverick

2 Answers

2
votes

Most likely you modify _payments variable from another thread while your TotalChange property code is being executed.

If this is true, try avoid multithread access to _payments variable or consider use locking while doing read/write access to _payments variable

lock(_lockerObject)
{
    // Do operations that modify _payments collection
    _payments.Add();//...
}

    public decimal TotalChange
    {
        get
        {
            lock (_lockerObject)
            {
                var change = (from IPayment payment in _payments where payment.Amount < 0 select payment.Amount).Sum();
                var paid = (from IPayment payment in _payments select payment.Amount).Sum();
            }
            return paid <= Total && Total > 0 ? Math.Abs(change) : paid - Total;
        }
    }
1
votes

There are many ways to solve this, most simple one is to work on another list, so you can add _payments:

public decimal TotalChange
{
    get
    {
        lock(_syncObj)
        {
           var p = _payments.ToList();
        }            

        var change = (from IPayment payment in p where payment.Amount < 0 select payment.Amount).Sum();
        var paid = (from IPayment payment in p select payment.Amount).Sum();
        return paid <= Total && Total > 0 ? Math.Abs(change) : paid - Total;
    }
}

And you also need to lock it when you add items:

lock(_syncObj)
{
    _payments.Add();//...
}