Properly locking a List<T> in MultiThreaded Scenarios?

.net, c#, multithreading

Solution

I will lock on the _myList itself here since it is private, but using a separate variable is more common. To improve on a few points:

public static class MyClass 
{
    private static List<string> _myList = new List<string>;
    private static bool _record; 

    public static void StartRecording()
    {
        lock(_myList)   // lock on the list
        {
           _myList.Clear();
           _record = true;
        }
    }

    public static IEnumerable<string> StopRecording()
    {
        lock(_myList)
        {
          _record = false;
          // Return a Read-Only copy of the list data
          var result = new List<string>(_myList).AsReadOnly();
          _myList.Clear();
          return result;
        }
    }

    public static void DoSomething()
    {
        lock(_myList)
        {
          if(_record) _myList.Add("Test");
        }
        // More, but unrelated actions
    }
}

Note that this code uses `lock(_myList)` to synchronize access to both _myList and _record. And you need to sync all actions on those two.

And to agree with the other answers here, `lock(_myList)` does nothing to _myList, it just uses _myList as a token (presumably as key in a HashSet). All methods must play fair by asking permission using the same token. A method on another thread can still use _myList without locking first, but with unpredictable results.

We can use any token so we often create one specially:

private static object _listLock = new object();

And then use `lock(_listLock)` instead of `lock(_myList)` everywhere.

This technique would have been advisable if myList had been public, and it would have been absolutely necessary if you had re-created myList instead of calling Clear().

Problem

Okay, I just can't get my head around multi-threading scenarios properly. Sorry for asking a similar question again, I'm just seeing many different "facts" around the internet. ``` public static class MyClass { private static List<string> _myList = new List<string>; private static bool _record; public static void StartRecording() { _myList.Clear(); _record = true; } public static IEnumerable<string> StopRecording() { _record = false; // Return a Read-Only copy of the list data var result = new List<string>(_myList).AsReadOnly(); _myList.Clear(); return result; } public static void DoSomething() { if(_record) _myList.Add("Test"); // More, but unrelated actions } } ``` The idea is that if Recording is activated, calls to DoSomething() get recorded in an internal List, and returned when StopRecording() is called. My specification is this: - StartRecording is not considered Thread-Safe. The user should call this while no other Thread is calling DoSomething(). But if it somehow could be, that would be great. - StopRecording is also not officially thread-safe. Again, it would be great if it could be, but that is not a requirement. - DoSomething has to be thread-safe The usual way seems to be: ``` public static void DoSomething() { object _lock = new object(); lock(_lock){ if(_record) _myList.Add("Test"); } // More, but unrelated actions } ``` Alternatively, declaring a static variable: ``` private static object _lock; public static void DoSomething() { lock(_lock){ if(_record) _myList.Add("Test"); } // More, but unrelated actions } ``` However, this answer says that this does not prevent other code from accessing it. So I wonder - How would I properly lock a list? - Should I create the lock object in my function or as a static class variable? - Can I wrap the functionality of Start and StopRecording in a lock-block as well? - StopRecording() does two things: Set a boolean variable to false (to prevent DoSomething() from adding more stuff) and then copying the list to return a copy of the data to the caller). I assume that _record = false; is atomic and will be in effect immediately? So normally I wouldn't have to worry about Multi-Threading here at all, unless some other Thread calls StartRecording() again? At the end of the day, I am looking for a way to express "Okay, this list is mine now, all other threads have to wait until I am done with it".

Original source

Related problems