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

Question

I am wondering how to correctly getter 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, the first class has an open list, which uses the second class. The first class periodically updates the list in one thread, and the second class reads the list in an arbitrary interval in the second thread.

Does this locking mechanism ensure that the list is not changed by the first class while the second class reads it and vice versa?

Answer 1

No, it is not.

All that ensures that the list does not change when you return it from the property.
Since access to the field is already guaranteed to be atomic, this is a completely useless lock.

You need to put a lock around the entire foreach .

---

Note that you can achieve better performance with ReaderWriterLockSlim and even improve performance with ConcurrentBag<T> without any locks.

Answer 2

No, it is not. In particular, the first blocked code shown protects only the receipt of a link to the list . After that, other threads can compete while the caller does ... well, whatever they do after accessing my_list .

Answer 3

You can be safe with a copy:

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

It depends on your requirement, if acceptable.

Answer 4

Perhaps instead of locking your lists, you can open them as ReadOnlyCollections, and also provide a method to add to lists that process logic.

Answer 5

No, your reader only blocks the list for the duration of access to the resource. You need to block the list around the entire foreach loop if you need it.

Answer 6

No, the lock you have is completely useless. It will protect only access to the variable containing the list, but as soon as you get a copy of this link, access to the list is not completely protected.

You should not have a property that returns a link to the list, you should have methods that do what you need with the list:

 class FirstClass { static private locker = new object(); static private List<String> _my_list; public static void Add(string item) { lock(locker) { my_list.Add(item); } } public static void RemoveAt(int index) { lock(locker) { my_list.RenoveAt(index); } } public static IEnumerable<string> GetEnumerable() { lock(locker) { List<string> copy = new List<string>(my_list); return copy; } } }