CODEWITHSUNDEEP
c#multithreadinglistreferencelocking
nv dev
nv dev

Reputation: 41

Do locks ensure the thread safety of list elements?

To my understanding, they do not. Please confirm.

Suppose I have a Processor object that contains a List<T> where T is a reference type.

The UI thread instantiates the Processor object, and periodically calls into it to get the List<T>.

The processor object also starts a task that has a reference to the List<T> when it is constructed.

A lock is used to reserve access to the List<T> in the getter and in the task to ensure they have exclusive access to the List<T>.

public class Processor
{
  private List<T> _list = new List<T>();
  private Object _listLock = new Object();

  public Processor()
  {
    // populate _list

    Task t = new Task(Process);

    t.Start();
  }

  public List<T> GetList()
  {
    lock(_listLock)
    {
      return _list;
    }
  }

  private void Process()
  {
    while(!doneProcessing)
    {
      lock(_listLock)
      {
        // access and modify _list items
      }

      Thread.Sleep(...);
    }
  }
}

But even if List<T> is locked in the getter, and it returned the list reference without issue, the task started by the Processor is still modifying the reference type list elements when it seizes the lock.

The elements of the list are still subject to change from the Processor’s task, and accessing them in the UI thread would not be thread safe.

If I am correct, an obvious solution is to have the getter return a new list populated with deep copies of the list elements.

public List<T> GetList()
{
  lock(_listLock)
  {
    return _list.Select(t => t.Clone()).ToList();
  }
}

What else can you do?

Upvotes: 0

Views: 161

Answers (2)

John Wu
John Wu

Reputation: 52240

Locks, used the way you have, will not ensure thread safety.

If you're trying for thread safety, consider using one of the thread safe collections available in the .NET CLR.

You'll notice there is no thread-safe IList. Here's why. The very notion of a thread safe list doesn't make a lot of sense. But with some minor design changes you could easily use something like a ConcurrentDictionary or ConcurrentBag.

Upvotes: 2

saille
saille

Reputation: 9191

Your GetList() doesn't do what you think it does:

public List<T> GetList()
  {
    lock(_listLock)
    {
      return _list;
    }
  }

Since GetList() just returns a reference to _list, the lock() does nothing except prevent 2 threads from getting a reference to _list at the same time, which is not an issue anyway.

The issue is that you are passing a reference to a list object back to the UI thread, and the elements in the list pointed to by the reference could change at any time, event while the UI thread is iterating over the elements, which is not good.

You could expose the lock object to the UI thread, but this means your UI will need to block while the list is being updated, which is usually undesirable (blocking UI thread degrades user experience).

Depending on what your UI does with the list, it might be better to take an immutable snapshot of your list and return that to the UI.

If you are wanting to keep the UI in step with changes to the underlying list data, then the approach you take really depends on the UI technology.

Upvotes: 0

Related Questions