ASP.NET HostedService thread safe singleton

Question

Do I need to care about thread safety when read/writing to items inside a List used inside a singleton.
If so, do I need to lock the getter/setter of Foo.Value or FooManager.FooList?

I know that reading and adding items to a List is not thread-safe, but I'm unsure if this also applies to my code.

Example code:

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
                
public class Program
{
  public static void Main()
  {
    var manager = new FooManager();
    manager.FooList.Add(new Foo());
    manager.FooList.Add(new Foo());
    manager.FooList.Add(new Foo());
    
    var updater = new FooUpdater(manager);
    updater.UpdateAsync();
  }
}

public class FooManager // Singleton
{
  public List<Foo> FooList {get;set;}
  
  public FooManager()
  {
    FooList = new List<Foo>();
  }
}

public class Foo
{
  public string Value {get; set;}

  public async Task UpdateValue(string value)
  {
    await Task.Delay(10000);
    Value = value;
  }
}

public class FooUpdater // HostedService
{
  private readonly FooManager _foomanager;

  public FooUpdater(FooManager foomanager)
  {
    _foomanager = foomanager;
  }

  public async Task UpdateAsync()
  {
    while(true)
    {
        foreach(var foo in _foomanager.FooList)
        {
            await foo.UpdateValue("AAA");
        }
    }
  }
}

public class FooController // Api Controller
{
  private readonly FooManager _fooManager;

  public FooController(FooManager foomanager)
  {
    _fooManager = foomanager;
  }

  // ...
  // Read or write to foomanager.FooList items value
}

`List` are not thread safe - so adding/removing items is not safe. if only one thing is operating on the objects referenced by the list, then it should be safe. — Daniel A. White, Nov 11 '21 at 01:45
@DanielA.White Since the `FooUpdater` is a `HostedService` running inside a seperate task, more than one (the service and the controller) can access the objects inside the `FooManger`. Correct? — rosi97, Nov 11 '21 at 01:47
If you are updating list from multiple threads then you don't know the order, so how will you read it (Or) will you read all the items at once? — Vijay Nirmal, Nov 11 '21 at 10:10
@Vijay Nirmal The FooManager will update the objects inside the list in the while loop. The Controller can access these values if necessary, using a Guid/ID of the Foo objects. (GUID/ID is not present in the example code, but will be in real life code) — rosi97, Nov 11 '21 at 21:38
So for my understanding there is chance that the controller can read a FooList object while it’s being written by the FooUpdater. My question is now, where to put the lock - when accessing the list or at the value of a object. — rosi97, Nov 11 '21 at 21:40

Vijay Nirmal · Accepted Answer · 2021-11-14T16:36:42.633

3

There are 2 potential thread-safe issue in your code

Writing/Updating to List<T> from multiple threads at the same time.

Solution: Use ConcurrentQueue<T> (see @TheodorZoulias comment below) instead of List<T>, since you won't access the items via index this is a perfect fit.
There is a chance that the same foo object is updated (foo.UpdateValue(string)) from HostedService and FooController at the same time.

Solution: You need to make UpdateValue method in foo class thread-safe. Sorry, I can't provide you sample code since you didn't provide the actual (simpler to actual) code in the post.

If so, do I need to lock the getter/setter of Foo.Value or FooManager.FooList?

Locking getter/setter of FooManager.FooList has no use. You should lock Foo.Value if the type wasn't a type with atomic reads/writes (eg. double), better lock for all types, but I don't think value will be corrupted if we didn't lock, so personally I usually won't lock since I don't care about value consistency among threads.

edited Nov 14 '21 at 16:36

answered Nov 12 '21 at 14:07

Vijay Nirmal

5,239
4
26
59

A `ConcurrentBag` [is unlikely to be the best solution](https://stackoverflow.com/questions/15400133/when-to-use-blockingcollection-and-when-concurrentbag-instead-of-listt/64823123#64823123) for this problem. If the order of the items stored inside the collection matters, then a `ConcurrentQueue` should be preferable. – Theodor Zoulias Nov 12 '21 at 15:02
2

@TheodorZoulias I have asked him in the comments, He is not worried about the ordering. Anyway I have updated the answer with `ConcurrentQueue` as a option – Vijay Nirmal Nov 14 '21 at 07:25
1

Vijay even if preserving the order is not required for the final app, it might be useful during the debugging of the app. And it comes at no cost. The `ConcurrentQueue` is not less efficient that the `ConcurrentBag`. The only inconvenience is having to write `Enqueue` instead of `Add`. – Theodor Zoulias Nov 14 '21 at 13:47

ASP.NET HostedService thread safe singleton

1 Answers1