Description
Description
The file contains the class ProgressHandler whose methods AddOperation, RemoveOperation, and UpdateOperation may be called in parallel judging from the use of ConcurrentDictionary. All three methods invoke UpdateTaskbarProgress which ultimately uses the property Progress:
Files/Files.Launcher/MessageHandlers/FileOperationsHandler.cs
Lines 809 to 816 in 3a8da90
The property makes use of the field operations of type ConcurrentDictionary. Now, to compute the actual progress, it uses a combination of LINQ-Statements. I assume the intent of ToList here is to create a snapshot/copy of the operations dictionary to run the Where method on it safely. Unfortunately, the ToList method uses an optimized copy-mechanism for collections that implement the ICollection interface, which ConcurrentDictionary does. It first retrieves the length of the source collection, initializes the internal data structure accordingly, and copies the values after that; thus, there’s a race condition between the initialization and the copy-mechanism. You can see this behavior within the reference source:
Steps To Reproduce
Due to the nature of race conditions, it's hard to reproduce the error in the actual code (and probably unlikely to occur). However, you can reproduce the race condition with the following example:
public class Program {
private static ConcurrentDictionary<int, int> dictionary = new();
static async Task Main() {
Console.WriteLine("starting");
var producer = Task.Run(Produce);
CreateToListCopies();
await producer;
Console.WriteLine("completed");
}
private static void Produce() {
for(int i = 0; i < 100_000_000; i++) {
dictionary.TryAdd(i, i);
}
Console.WriteLine("finished producing");
}
private static void CreateToListCopies() {
for(int i = 0; i < 100_000_000; i++) {
dictionary.ToList(); // race condition that leads to System.ArgumentException
//dictionary.ToArray(); // no race condition, ToArray() correctly creates a snapshot
}
Console.WriteLine("finished copying");
}
}Expected behavior
The straightforward fix is replacing the invocation of LINQ’s ToList with ToArray implementation of ConcurrentDictionary. Alternatively, you can apply ToList after the Where clause to not have a race condition between Any and Average (GetEnumerator does not create a snapshot but is thread-safe).
Files Version
latest commit to the main branch: fe684bc; latest commit to the document: 3a8da90
Windows Version
any
Relevant Assets/Logs
none