mirror of
https://github.com/JustArchiNET/ArchiSteamFarm
synced 2024-11-13 00:17:21 +00:00
b6805a94a3
This is some next-level race condition, so for those interested: - Concurrent collections are thread-safe in a way that each operation is atomic - Naturally if you call two atomic operations in a row, the result is no longer atomic, since there could be some changes between the first and the last - Certain LINQ operations such as OrderBy(), Reverse(), ToArray(), among more, use internal buffer for operation with certain optimization that checks if input is ICollection, if yes, it calls Count and CopyTo(), for OrderBy in this example - In result, such LINQ call is not guaranteed to be thread-safe, since it assumes those two calls to be atomic, while they're not in reality. This issue is quite hard to spot in real applications, since it's not that easy to trigger it (you need to call the operation on ICollection and then have another thread modifying it while enumerating). This is probably why we've never had any real problem until I've discovered this madness with @Aareksio in entirely different project. As a workaround, we'll explicitly convert some ICollection inputs to IEnumerable, in particular around OrderBy(), so the optimization is skipped and the result is not corrupted. I've added unit tests which ensure this workaround works properly, and you can easily reproduce the problem by removing AsLinqThreadSafeEnumerable() in them. See https://github.com/dotnet/runtime/discussions/50687 for more insight I have no clue who thought that ignoring this issue is a good idea, at the very least concurrent collections should have opt-out mechanism from those optimizations, there is no reason for them to not do that.
93 lines
3 KiB
C#
93 lines
3 KiB
C#
// ----------------------------------------------------------------------------------------------
|
|
// _ _ _ ____ _ _____
|
|
// / \ _ __ ___ | |__ (_)/ ___| | |_ ___ __ _ _ __ ___ | ___|__ _ _ __ _ __ ___
|
|
// / _ \ | '__|/ __|| '_ \ | |\___ \ | __|/ _ \ / _` || '_ ` _ \ | |_ / _` || '__|| '_ ` _ \
|
|
// / ___ \ | | | (__ | | | || | ___) || |_| __/| (_| || | | | | || _|| (_| || | | | | | | |
|
|
// /_/ \_\|_| \___||_| |_||_||____/ \__|\___| \__,_||_| |_| |_||_| \__,_||_| |_| |_| |_|
|
|
// ----------------------------------------------------------------------------------------------
|
|
// |
|
|
// Copyright 2015-2024 Łukasz "JustArchi" Domeradzki
|
|
// Contact: JustArchi@JustArchi.net
|
|
// |
|
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
|
// you may not use this file except in compliance with the License.
|
|
// You may obtain a copy of the License at
|
|
// |
|
|
// http://www.apache.org/licenses/LICENSE-2.0
|
|
// |
|
|
// Unless required by applicable law or agreed to in writing, software
|
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
// See the License for the specific language governing permissions and
|
|
// limitations under the License.
|
|
|
|
using System.Collections.Concurrent;
|
|
using System.Collections.Generic;
|
|
using System.Linq;
|
|
using ArchiSteamFarm.Collections;
|
|
using ArchiSteamFarm.Core;
|
|
using Microsoft.VisualStudio.TestTools.UnitTesting;
|
|
|
|
namespace ArchiSteamFarm.Tests;
|
|
|
|
#pragma warning disable CA1812 // False positive, the class is used during MSTest
|
|
[TestClass]
|
|
internal sealed class ConcurrentTests {
|
|
[TestMethod]
|
|
internal void ConcurrentDictionarySupportsWritingDuringLinq() {
|
|
ConcurrentDictionary<ushort, bool> collection = [];
|
|
|
|
for (byte i = 0; i < 10; i++) {
|
|
collection.TryAdd(i, true);
|
|
}
|
|
|
|
Utilities.InBackground(
|
|
() => {
|
|
for (ushort i = 10; i < ushort.MaxValue; i++) {
|
|
collection.TryAdd(i, true);
|
|
}
|
|
}, true
|
|
);
|
|
|
|
foreach (KeyValuePair<ushort, bool> _ in collection.AsLinqThreadSafeEnumerable().OrderBy(static entry => entry.Key)) { }
|
|
}
|
|
|
|
[TestMethod]
|
|
internal void ConcurrentHashSetSupportsWritingDuringLinq() {
|
|
ConcurrentHashSet<ushort> collection = [];
|
|
|
|
for (byte i = 0; i < 10; i++) {
|
|
collection.Add(i);
|
|
}
|
|
|
|
Utilities.InBackground(
|
|
() => {
|
|
for (ushort i = 10; i < ushort.MaxValue; i++) {
|
|
collection.Add(i);
|
|
}
|
|
}, true
|
|
);
|
|
|
|
foreach (ushort _ in collection.AsLinqThreadSafeEnumerable().OrderBy(static entry => entry)) { }
|
|
}
|
|
|
|
[TestMethod]
|
|
internal void ConcurrentListSupportsWritingDuringLinq() {
|
|
ConcurrentList<ushort> collection = [];
|
|
|
|
for (byte i = 0; i < 10; i++) {
|
|
collection.Add(i);
|
|
}
|
|
|
|
Utilities.InBackground(
|
|
() => {
|
|
for (ushort i = 10; i < ushort.MaxValue; i++) {
|
|
collection.Add(i);
|
|
}
|
|
}, true
|
|
);
|
|
|
|
foreach (ushort _ in collection.AsLinqThreadSafeEnumerable().OrderBy(static entry => entry)) { }
|
|
}
|
|
}
|
|
#pragma warning restore CA1812 // False positive, the class is used during MSTest
|