Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Inconsistent behavior: no exception is thrown in the List<T>.Sort method when called in a foreach loop

We know, if we change a collection in a foreach loop, the following exception is thrown:

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

But there is a method that behaves differently: List<T>.Sort(Comparison<T>).

For example (dotnetfiddle.net):

List<int> list = new List<int> { 2, 1 } ;
foreach (int i in list)
{
    //list.Sort(Comparer<int>.Default);         // InvalidOperationException
    //list.Sort();                              // InvalidOperationException
    list.Sort((a, b) => a.CompareTo(b));        // No exception

    Console.WriteLine(i);
}

According to the referencesource.microsoft.com we can see that there is no version increment in this particular method, while there is one in the method above:

public void Sort(int index, int count, IComparer<T> comparer) {
    ...
    _version++;
}

Version is incremented also in all other methods that modify the list.

My questions are:

  1. Is it a bug? Or there is some reason for this behavior?
  2. If it's a bug, why it hasn't been fixed for years?

As I remember I've posted this bug report to Microsoft about 8-9 years ago, but it was declined. And now I cannot find it anymore.

like image 591
Dmitry Avatar asked Jan 13 '17 13:01

Dmitry


1 Answers

It was a bug, that's why it is fixed in latest source code.

https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Collections/Generic/List.cs#L986

    public void Sort(Comparison<T> comparison) {
        if( comparison == null) {
            ThrowHelper.ThrowArgumentNullException(ExceptionArgument.comparison);
        }
        Contract.EndContractBlock();

        if (_size > 1) {
            ArraySortHelper<T>.Sort(_items, 0, _size, comparison);
        }
        _version++;
    }

There may be many similar bugs, unless someone reports and it is considered as very risky, they remain as they are not priorities.

Don't worry, there was similar bug in List.ForEach, that would not throw Modified Exception, I did report it and they declined, but in subsequent version it was fixed.

like image 139
Akash Kava Avatar answered Nov 15 '22 00:11

Akash Kava