0

I read pretty old code in System.Collections.Generic.SynchronizedCollection<T> and cannot understand, why items and sync fields don't have readonly modifier:

  public class SynchronizedCollection<T> : 
    IList<T>,
    ICollection<T>,
    IEnumerable<T>,
    IEnumerable,
    IList,
    ICollection
  {
    private List<T> items;
    private object sync;

I've checked both old codebase and new library with this code, and code is the same:

  1. https://github.com/microsoft/referencesource/blob/master/System.ServiceModel/System/ServiceModel/SynchronizedCollection.cs#L15
  2. https://github.com/dotnet/wcf/blob/main/src/System.Private.ServiceModel/src/System/ServiceModel/SynchronizedCollection.cs#L14

From my perspective there is no reason for not having readonly, but maybe I miss some tricky performance optimisation.

Denis
  • 3,595
  • 12
  • 52
  • 86
  • 2
    I doubt there's a reason, it's probably just old code that nobody uses anymore (there are much better concurrent collections available). If you want, you can sign up as a contributor and make a pull request to change it. – Charlieface Jul 24 '22 at 12:04
  • @Charlieface what is the modern `List` thread safe collection? – Denis Jul 24 '22 at 12:20
  • [`ConcurrentBag`](https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentbag-1?view=net-6.0) I think, admittedly that doesn't keep any overall ordering – Charlieface Jul 24 '22 at 12:22
  • @Charlieface yep, we cannot just replace List with Bag, if we need order. – Denis Jul 24 '22 at 12:33
  • @Charlieface I would like to [dissuade](https://stackoverflow.com/questions/15400133/when-to-use-blockingcollection-and-when-concurrentbag-instead-of-listt/64823123#64823123) you from thinking the `ConcurrentBag` as a thread-safe `List`! – Theodor Zoulias Jul 24 '22 at 12:42
  • Then just put a `lock` on a `List`. It would be difficult to find a more efficient implementation for a concurrent ordered list anyway. – Charlieface Jul 24 '22 at 12:50
  • Denis the API of the `List` is unsuitable for multithreaded usage. It makes race conditions unavoidable. For example you can't prevent the collection from containing more than N elements, with `if (list.Count < N) list.Add(x)`. This operation requires an atomic API, and the `Count`+`Add` is not atomic. With this code, it's quite likely that your list will have more than N elements at some point. The `SynchronizedCollection` suffers from the same problem, and that's why almost nobody uses it. – Theodor Zoulias Jul 24 '22 at 12:56
  • @TheodorZoulias well, if someone writes `if (list.Count < N) list.Add(x)`, they probably understand that they can get described behavior. Sometimes, you still want to keep some ordered events in a data structure (e.g., you collect ordered logs) – Denis Jul 24 '22 at 13:19
  • Denis if you intend to use exclusively the `SynchronizedCollection.Add` method, and also wait until all parallel operations have completed before enumerating the collection, then the `SynchronizedCollection` is OK. But in this scenario the `ConcurrentQueue` might be preferable. The [`Enqueue`](https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentqueue-1.enqueue) method adds at the end of the collection, just like the `List.Add`. – Theodor Zoulias Jul 24 '22 at 13:29
  • 1
    @TheodorZoulias yep, you are right, my example is not good, because for such case a queue will work fine for us. So it is about some case, when we want to have add + index access (it is hard to come up with such case, if we are talking about multithreading code, probably that's the reason, why queue and map are more popular data structures for multithreading programming). – Denis Jul 24 '22 at 14:43

0 Answers0