How to properly lock List<String> getter in C#

Question

I am wondering how to properly lock the getter of type List<String>. I have one static class that looks something like this:

class FirstClass {
static private locker = new object();   
static private List<String> _my_list;

public static List<String> my_list {
    get {
        lock(locker) {
            return my_list;
        }
    }
}

private static void thread_func() {
    // do something periodicaly with the list
    // for example:

    lock(locker){
        _my_list.Add();
        _my_list.RemoveAt();
        ...
    }
}

}

Then, I have another class that looks like this:

class SecondClass {
private void thread_func() {
    foreach(string s in FirstClass.my_list) {
        // read every item in the list
    }
}

}

So, first class has a public list that the second class uses. First class periodically updates the list in one thread, and second class reads the list at an random interval on a second thread.

Does this locking mechanism ensure that the list will not be modified by the first class while the second class is reading it and vice-versa?

Answer 1

No, it doesn't.

All that ensures is that the list isn't modified as you return it from the property.
Since field access is already guaranteed to be atomic, it's an utterly useless lock.

You need to put a lock around the entire foreach loop.

---

Note that you can achieve better performance by using a ReaderWriterLockSlim, and even better performance by using a ConcurrentBag<T> without any locks.

Answer 2

No, it does not. In particular the first lock shown only protects for the duration of obtaining the list reference. After that, other threads can compete while the caller does... well, whatever they do after accessing my_list.

Answer 3

You would be safe with a Copy:

public static List<String> my_list 
{
    get {
        lock(locker) {
            return my_list.ToList();
        }
    }
}

It depends on your req's if that's acceptable.