fix: Fixes map iterators for Items (#1564)

### Summary

Modifying a ValueLinkList using one of the methods will bump the "version". This field is used by iterators (foreach loops) to determine if the link list was modified while iterating. The sector.Items (and in the future other lists), will no longer be safe to modify while iterating. The server will _CRASH_ if the ValueLinkList is modified.

Thanks to @stefanomerotta for help!


### Screenshots
<img width="588" alt="image" src="https://github.com/modernuo/ModernUO/assets/3953314/83ee0b6e-ff4f-4768-9e29-84456e04b1ec">
This commit is contained in:
Kamron Batman 2023-10-26 17:49:15 -07:00 • committed by GitHub
parent 658f564f34
commit d919f71149
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
33 changed files with 706 additions and 468 deletions

View file

@ -1,3 +1,4 @@
using System;
using Server.Collections;
using Xunit;
@ -32,20 +33,35 @@ public class ValueLinkListTests
Assert.True(entity1.OnLinkList);
Assert.Equal(1, linkList.Count);
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity1, linkList.Last);
Assert.Collection(linkList.ToArray(), item =>
{
Assert.Equal(entity1, item);
Assert.Null(item.Previous);
Assert.Null(item.Next);
}
);
var entity2 = new TestEntity(2);
linkList.AddFirst(entity2);
Assert.True(entity2.OnLinkList);
Assert.Equal(2, linkList.Count);
Assert.Equal(entity1, linkList.Last);
Assert.Equal(entity2, entity1.Previous);
Assert.Equal(entity2, linkList.First);
Assert.Equal(entity1, entity2.Next);
Assert.Collection(linkList.ToArray(),
item =>
{
Assert.Equal(entity2, item);
Assert.Null(item.Previous);
Assert.Equal(item.Next, entity1);
},
item =>
{
Assert.Equal(entity1, item);
Assert.Equal(item.Previous, entity2);
Assert.Null(item.Next);
}
);
}
[Fact]
@ -58,8 +74,14 @@ public class ValueLinkListTests
Assert.True(entity1.OnLinkList);
Assert.Equal(1, linkList.Count);
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity1, linkList.Last);
Assert.Collection(linkList.ToArray(), item =>
{
Assert.Equal(entity1, item);
Assert.Null(item.Previous);
Assert.Null(item.Next);
}
);
var entity2 = new TestEntity(2);
@ -67,11 +89,21 @@ public class ValueLinkListTests
Assert.True(entity2.OnLinkList);
Assert.Equal(2, linkList.Count);
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity2, entity1.Next);
Assert.Equal(entity2, linkList.Last);
Assert.Equal(entity1, entity2.Previous);
Assert.Collection(linkList.ToArray(),
item =>
{
Assert.Equal(entity1, item);
Assert.Null(item.Previous);
Assert.Equal(item.Next, entity2);
},
item =>
{
Assert.Equal(entity2, item);
Assert.Equal(item.Previous, entity1);
Assert.Null(item.Next);
}
);
}
[Fact]
@ -87,20 +119,48 @@ public class ValueLinkListTests
Assert.True(entity1.OnLinkList);
Assert.True(entity2.OnLinkList);
Assert.Equal(2, linkList.Count);
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity2, linkList.Last);
Assert.Collection(linkList.ToArray(),
item =>
{
Assert.Equal(entity1, item);
Assert.Null(item.Previous);
Assert.Equal(item.Next, entity2);
},
item =>
{
Assert.Equal(entity2, item);
Assert.Equal(item.Previous, entity1);
Assert.Null(item.Next);
}
);
var entity3 = new TestEntity(3);
linkList.AddBefore(entity2, entity3);
Assert.True(entity3.OnLinkList);
Assert.Equal(3, linkList.Count);
Assert.Equal(entity1, entity3.Previous);
Assert.Equal(entity2, entity3.Next);
// First and Last should not have changed
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity2, linkList.Last);
Assert.Collection(linkList.ToArray(),
item =>
{
Assert.Equal(entity1, item);
Assert.Null(item.Previous);
Assert.Equal(item.Next, entity3);
},
item =>
{
Assert.Equal(entity3, item);
Assert.Equal(item.Previous, entity1);
Assert.Equal(item.Next, entity2);
},
item =>
{
Assert.Equal(entity2, item);
Assert.Equal(item.Previous, entity3);
Assert.Null(item.Next);
}
);
}
[Fact]
@ -113,23 +173,31 @@ public class ValueLinkListTests
linkList.AddFirst(entity1);
linkList.AddLast(entity2);
Assert.True(entity1.OnLinkList);
Assert.True(entity2.OnLinkList);
Assert.Equal(2, linkList.Count);
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity2, linkList.Last);
var entity3 = new TestEntity(3);
linkList.AddAfter(entity1, entity3);
Assert.True(entity3.OnLinkList);
Assert.Equal(3, linkList.Count);
Assert.Equal(entity1, entity3.Previous);
Assert.Equal(entity2, entity3.Next);
// First and Last should not have changed
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity2, linkList.Last);
Assert.Collection(linkList.ToArray(),
item =>
{
Assert.Equal(entity1, item);
Assert.Null(item.Previous);
Assert.Equal(item.Next, entity3);
},
item =>
{
Assert.Equal(entity3, item);
Assert.Equal(item.Previous, entity1);
Assert.Equal(item.Next, entity2);
},
item =>
{
Assert.Equal(entity2, item);
Assert.Equal(item.Previous, entity3);
Assert.Null(item.Next);
}
);
}
[Fact]
@ -157,12 +225,47 @@ public class ValueLinkListTests
Assert.Equal(1, linkList.Count);
Assert.Equal(5, linkList2.Count);
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity1, linkList.Last);
Assert.Collection(linkList.ToArray(),
item =>
{
Assert.Equal(entity1, item);
Assert.Null(item.Previous);
Assert.Null(item.Next);
}
);
Assert.Equal(entity2, entity6.Next);
Assert.Equal(entity6, entity2.Previous);
Assert.Equal(entity4, linkList2.Last);
Assert.Collection(linkList2.ToArray(),
item =>
{
Assert.Equal(entity5, item);
Assert.Null(item.Previous);
Assert.Equal(item.Next, entity6);
},
item =>
{
Assert.Equal(entity6, item);
Assert.Equal(item.Previous, entity5);
Assert.Equal(item.Next, entity2);
},
item =>
{
Assert.Equal(entity2, item);
Assert.Equal(item.Previous, entity6);
Assert.Equal(item.Next, entity3);
},
item =>
{
Assert.Equal(entity3, item);
Assert.Equal(item.Previous, entity2);
Assert.Equal(item.Next, entity4);
},
item =>
{
Assert.Equal(entity4, item);
Assert.Equal(item.Previous, entity3);
Assert.Null(item.Next);
}
);
}
[Fact]
@ -184,10 +287,20 @@ public class ValueLinkListTests
linkList.RemoveAllBefore(entity4);
Assert.Equal(2, linkList.Count);
Assert.Equal(entity4, linkList.First);
Assert.Equal(entity5, linkList.Last);
Assert.Null(entity4.Previous);
Assert.Null(entity5.Next);
Assert.Collection(linkList.ToArray(),
item =>
{
Assert.Equal(entity4, item);
Assert.Null(item.Previous);
Assert.Equal(item.Next, entity5);
},
item =>
{
Assert.Equal(entity5, item);
Assert.Equal(item.Previous, entity4);
Assert.Null(item.Next);
}
);
}
[Fact]
@ -209,9 +322,104 @@ public class ValueLinkListTests
linkList.RemoveAllAfter(entity2);
Assert.Equal(2, linkList.Count);
Assert.Equal(entity1, linkList.First);
Assert.Equal(entity2, linkList.Last);
Assert.Null(entity1.Previous);
Assert.Null(entity2.Next);
Assert.Collection(linkList.ToArray(),
item =>
{
Assert.Equal(entity1, item);
Assert.Null(item.Previous);
Assert.Equal(item.Next, entity2);
},
item =>
{
Assert.Equal(entity2, item);
Assert.Equal(item.Previous, entity1);
Assert.Null(item.Next);
}
);
}
[Fact]
public void TestVersionIncrements()
{
var linkList = new ValueLinkList<TestEntity>();
var entity1 = new TestEntity(1);
var entity2 = new TestEntity(2);
var entity3 = new TestEntity(3);
var entity4 = new TestEntity(4);
var version = 0;
Assert.Equal(version, linkList.Version);
linkList.AddFirst(entity1); // 1
Assert.Equal(++version, linkList.Version);
linkList.AddLast(entity2); // 1, 2
Assert.Equal(++version, linkList.Version);
linkList.AddBefore(entity2, entity3); // 1, 3, 2
Assert.Equal(++version, linkList.Version);
linkList.AddAfter(entity3, entity4); // 1, 3, 4, 2
Assert.Equal(++version, linkList.Version);
linkList.Remove(entity1); // 3, 4, 2
Assert.Equal(++version, linkList.Version);
linkList.RemoveAllAfter(entity4); // 3, 4
Assert.Equal(++version, linkList.Version);
linkList.RemoveAllBefore(entity4); // 4
Assert.Equal(++version, linkList.Version);
linkList.RemoveAll(); // None
Assert.Equal(++version, linkList.Version);
}
[Fact]
public void TestThrowsIfModifiedWhileIterating()
{
Assert.Throws<InvalidOperationException>(
() =>
{
var linkList = new ValueLinkList<TestEntity>();
var entity1 = new TestEntity(1);
var entity2 = new TestEntity(2);
linkList.AddFirst(entity1);
linkList.AddLast(entity2);
foreach (var item in linkList)
{
linkList.Remove(item);
}
}
);
}
[Fact]
public void TestThrowsIfModifiedWhileIteratingNested()
{
Assert.Throws<InvalidOperationException>(
() =>
{
var linkList = new ValueLinkList<TestEntity>();
var entity1 = new TestEntity(1);
var entity2 = new TestEntity(2);
linkList.AddFirst(entity1);
linkList.AddLast(entity2);
foreach (var item in linkList)
{
foreach (var nestedItem in linkList)
{
linkList.Remove(nestedItem);
}
}
}
);
}
}