Is it thread-safe to iterate over an immutable copy of Dictionary values if this copy was made under a lock?

Question

Is the below code thread-safe?

I need to call an async method on every service, therefore I cannot keep the foreach loop under the lock.

But would it be thread-safe to copy all the values from the _dictionary to an ImmutableList under the lock, exit the lock and then iterate over them as usual and call the async method?

public class Cache
{
    private readonly ReaderWriterLockSlim lockObject = new();

    private readonly IDictionary<string, Service> _dictionary = new Dictionary<string, Service>();

    public Service GetOrAdd(string key)
    {
        lockObject.EnterUpgradeableReadLock();
        try
        {
            if (_dictionary.TryGetValue(key, out var service))
            {
                return service;
            }
            lockObject.EnterWriteLock();
            try
            {
                var value = new Service();
                _dictionary.Add(key, value);
                return service;
            }
            finally
            {
                lockObject.ExitWriteLock();
            }
        }
        finally
        {
            lockObject.ExitUpgradeableReadLock();
        }
    }

    public async Task CallServices()
    {
        var services = GetServices();
        foreach (var service in services)
        {
            await service.DoStuffAsync();
        }
    }

    private ImmutableList<Service> GetServices()
    {
        lockObject.EnterReadLock();
        try
        {
            return _dictionary.Values.ToImmutableList();
        }
        finally
        {
            lockObject.ExitReadLock();
        }
    }
}

Replacing the Dictionary with the ConcurrentDictionary and removing the lock from the GetServices is one option, but I still have to keep the lock in GetOrAdd because in realilty I have 2 collections to maintain, not only the _dictionary. I was wondering if it was possible to use a normal Dictionary.

Answer 1

As far as I can tell, yes.

Since the copy of the dictionary is made while the lock is held there should be no risk that the dictionary is read and written to concurrently. And concurrent reads are safe:

A Dictionary<TKey,TValue> can support multiple readers concurrently, as long as the collection is not modified

The created copy is a local variable, and cannot be accessed by multiple threads, so using this is thread safe by default. There is not even any need to make it immutable, using a regular list would work just as well.

But this is specifically for the accessing the dictionary. .DoStuffAsync() could still be unsafe, but I assume that is out of scope.