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
}

CodePudding user response：

There are 2 potential thread-safe issue in your code

1. 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.

2. 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.