Add workaround for LINQ race condition with concurrent collections

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 dotnet/runtime#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.

ArchiSteamFarm.Tests/ConcurrentTests.cs

@@ -0,0 +1,93 @@
+// ----------------------------------------------------------------------------------------------
+//     _                _      _  ____   _                           _____
+//    / \    _ __  ___ | |__  (_)/ ___| | |_  ___   __ _  _ __ ___  |  ___|__ _  _ __  _ __ ___
+//   / _ \  | '__|/ __|| '_ \ | |\___ \ | __|/ _ \ / _` || '_ ` _ \ | |_  / _` || '__|| '_ ` _ \
+//  / ___ \ | |  | (__ | | | || | ___) || |_|  __/| (_| || | | | | ||  _|| (_| || |   | | | | | |
+// /_/   \_\|_|   \___||_| |_||_||____/  \__|\___| \__,_||_| |_| |_||_|   \__,_||_|   |_| |_| |_|
+// ----------------------------------------------------------------------------------------------
+// |
+// Copyright 2015-2024 Łukasz "JustArchi" Domeradzki
+// Contact: [email protected]
+// |
+// 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

ArchiSteamFarm/Core/Utilities.cs

@@ -55,6 +55,14 @@ public static class Utilities {
 
 	private static readonly FrozenSet<char> DirectorySeparators = new HashSet<char>(2) { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }.ToFrozenSet();
 
+	[PublicAPI]
+	public static IEnumerable<T> AsLinqThreadSafeEnumerable<T>(this IEnumerable<T> enumerable) {
+		ArgumentNullException.ThrowIfNull(enumerable);
+
+		// See: https://github.com/dotnet/runtime/discussions/50687
+		return enumerable.Select(static entry => entry);
+	}
+
 	[PublicAPI]
 	public static string GenerateChecksumFor(byte[] source) {
 		ArgumentNullException.ThrowIfNull(source);

ArchiSteamFarm/Steam/Bot.cs

@@ -572,7 +572,7 @@ public EAccess GetAccess(ulong steamID) {
 				case "@ALL":
 				case SharedInfo.ASF:
 					// We can return the result right away, as all bots have been matched already
-					return Bots.OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value).ToHashSet();
+					return Bots.AsLinqThreadSafeEnumerable().OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value).ToHashSet();
 				case "@FARMING":
 					IEnumerable<Bot> farmingBots = Bots.Where(static bot => bot.Value.CardsFarmer.NowFarming).OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value);
 					result.UnionWith(farmingBots);
@@ -604,7 +604,7 @@ public EAccess GetAccess(ulong steamID) {
 					switch (botRange.Length) {
 						case 1:
 							// Either bot.. or ..bot
-							IEnumerable<Bot> query = Bots.OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value);
+							IEnumerable<Bot> query = Bots.AsLinqThreadSafeEnumerable().OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value);
 
 							query = botName.StartsWith("..", StringComparison.Ordinal) ? query.TakeWhile(bot => bot != firstBot) : query.SkipWhile(bot => bot != firstBot);
 
@@ -620,7 +620,7 @@ public EAccess GetAccess(ulong steamID) {
 							Bot? lastBot = GetBot(botRange[1]);
 
 							if ((lastBot != null) && (BotsComparer.Compare(firstBot.BotName, lastBot.BotName) <= 0)) {
-								foreach (Bot bot in Bots.OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value).SkipWhile(bot => bot != firstBot).TakeWhile(bot => bot != lastBot)) {
+								foreach (Bot bot in Bots.AsLinqThreadSafeEnumerable().OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value).SkipWhile(bot => bot != firstBot).TakeWhile(bot => bot != lastBot)) {
 									result.Add(bot);
 								}
 
@@ -1331,7 +1331,7 @@ internal static string FormatBotResponse(string response, string botName) {
 			return targetBot;
 		}
 
-		return Bots.OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value).FirstOrDefault();
+		return Bots.AsLinqThreadSafeEnumerable().OrderBy(static bot => bot.Key, BotsComparer).Select(static bot => bot.Value).FirstOrDefault();
 	}
 
 	internal Task<HashSet<uint>?> GetMarketableAppIDs() => ArchiWebHandler.GetAppList();

ArchiSteamFarm/Steam/Cards/CardsFarmer.cs

@@ -811,7 +811,7 @@ private async Task Farm() {
 					// In order to maximize efficiency, we'll take games that are closest to our HoursPlayed first
 
 					// We must call ToList() here as we can't remove items while enumerating
-					foreach (Game game in GamesToFarm.OrderByDescending(static game => game.HoursPlayed).ToList()) {
+					foreach (Game game in GamesToFarm.AsLinqThreadSafeEnumerable().OrderByDescending(static game => game.HoursPlayed).ToList()) {
 						if (!await IsPlayableGame(game).ConfigureAwait(false)) {
 							GamesToFarm.Remove(game);
 
@@ -1423,7 +1423,7 @@ private bool ShouldIdle(uint appID) {
 
 	private async Task SortGamesToFarm() {
 		// Put priority idling appIDs on top
-		IOrderedEnumerable<Game> orderedGamesToFarm = GamesToFarm.OrderByDescending(game => Bot.IsPriorityIdling(game.AppID));
+		IOrderedEnumerable<Game> orderedGamesToFarm = GamesToFarm.AsLinqThreadSafeEnumerable().OrderByDescending(game => Bot.IsPriorityIdling(game.AppID));
 
 		foreach (BotConfig.EFarmingOrder farmingOrder in Bot.BotConfig.FarmingOrders) {
 			switch (farmingOrder) {