sestoft / C5

C5 generic collection library for C#/.NET
http://www.itu.dk/research/c5/
MIT License
1.03k stars 181 forks source link

modifycheck failed on Feb nuget and not Dec nuget #42

Closed Dana-Ferguson closed 8 years ago

Dana-Ferguson commented 8 years ago

Result StackTrace:

at C5.CollectionBase`1.modifycheck(Int32 thestamp)
   at C5.TreeSet`1.Enumerator.MoveNext()
   at C5.DictionaryBase`2.ValuesCollection.ValueEnumerator.MoveNext()
   at System.Linq.Enumerable.LastOrDefault[TSource](IEnumerable`1 source)

When I updated my C5 nuget from Dec 16th to Feb 17th (first of the day) I get this error when running unit tests.

EDIT: See next comment for an example.

The collection being used is TreeDictionary<,>. Order of events: TreeDictionary uses default empty constructor. TreeDictionary.Contains() called twice, returning false each time. TreeDictionary.UpdateOrAdd() is called 10 times. Successful add each time.

Values.Count() and Keys.Count() is queried once each.

Values.LastOrDefault is called once. UpdateOrAdd() is called again.

Values.LastOrDefault() is called again. This call then throws the exception.

I decided to look through the github changes (there are lots of non-code changes), and then a whole bunch of 'Sorry, we could not display the changes to this file because there were too many other changes to display.'

I'm going to keep looking, if I find the cause I'll post back here.

Dana-Ferguson commented 8 years ago

This is the minimal working example of the bug.

        var tree = new TreeDictionary<int, int>();

        tree.Values.LastOrDefault();
        tree.UpdateOrAdd(0, 0);
        tree.Values.LastOrDefault();

Yeap, that's weird.

Dana-Ferguson commented 8 years ago

If I'm getting the code right (maybe?), the internal stamp value starts at 0. Becomes 1 when tree.UpdateOrAdd(0, 0); is called, but when LastOrDefault() is called again, it performs the check, but sends 0, not 1.

ddur commented 8 years ago

Looks like it is same issue as #41. In your example you are calling Values twice.

Dana-Ferguson commented 8 years ago

Yeap, I agree. I looked over the issues first, didn't realize that it was already there, because I was looking for the exception.

ondfisk commented 8 years ago

Yep same as #41. I will look into it ASAP. I have hidden the buggy NuGet packages.