CODEWITHSUNDEEP
c#multithreadinglistlocking
Prix
Prix

Reputation: 19528

Using a List from 2 different threads?

I have a list where it is entries can be updated, new data inserted or removed from 2 different threads.

Is it ok to use a public readonly object to lock when it is being used to interact to the other thread as to when it is locked or not or what would be the correct way to use this list across the 2 threads ?

Upvotes: 3

Views: 6147

Answers (3)

Lazlo
Lazlo

Reputation: 8770

First, read this article to understand why it's bad: http://blogs.msdn.com/b/jaredpar/archive/2009/02/11/why-are-thread-safe-collections-so-hard.aspx

Then, do it anyway like I did:

public abstract class ConcurrentCollection<T> : ICollection<T>
{
    private List<T> List { get; set; }

    public ConcurrentCollection()
    {
        this.List = new List<T>();
    }

    public T this[int index]
    {
        get
        {
            return this.List[index];
        }
    }

    protected virtual void AddUnsafe(T item)
    {
        this.List.Add(item);
    }

    protected virtual void RemoveUnsafe(T item)
    {
        this.List.Remove(item);
    }

    protected virtual void ClearUnsafe()
    {
        this.List.Clear();
    }

    public void Add(T item)
    {
        lock (this.List)
        {
            this.AddUnsafe(item);
        }
    }

    public bool Remove(T item)
    {
        lock (this.List)
        {
            this.RemoveUnsafe(item);
            return true;
        }
    }

    public void Clear()
    {
        lock (this.List)
        {
            this.ClearUnsafe();
        }
    }

    public int Count
    {
        get
        {
            lock (this.List)
            {
                return this.List.Count;
            }
        }
    }

    public bool IsReadOnly
    {
        get
        {
            return false;
        }
    }

    public bool Contains(T item)
    {
        lock (this.List)
        {
            return this.List.Contains(item);
        }
    }

    public void CopyTo(T[] array, int arrayIndex)
    {
        lock (this.List)
        {
            this.List.CopyTo(array, arrayIndex);
        }
    }

    public IEnumerator<T> GetEnumerator()
    {
        return new ConcurrentEnumerator<T>(this.List, this.List);
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        throw new NotImplementedException("Abstract concurrent enumerators not implemented.");
    }
}

public class ConcurrentEnumerator<T> : IEnumerator<T>
{
    private int Position = -1;
    private List<T> Duplicate;
    private object Mutex;
    private ICollection<T> NonConcurrentCollection;

    internal ConcurrentEnumerator(ICollection<T> nonConcurrentCollection, object mutex)
    {
        this.NonConcurrentCollection = nonConcurrentCollection;
        this.Mutex = mutex;

        lock (this.Mutex)
        {
            this.Duplicate = new List<T>(this.NonConcurrentCollection);
        }
    }

    public T Current
    {
        get
        {
            return this.Duplicate[this.Position];
        }
    }

    object IEnumerator.Current
    {
        get
        {
            return this.Current;
        }
    }

    public bool MoveNext()
    {
        this.Position++;

        lock (this.Mutex)
        {
            while (this.Position < this.Duplicate.Count && !this.NonConcurrentCollection.Contains(this.Current))
            {
                this.Position++;
            }
        }

        return this.Position < this.Duplicate.Count;
    }

    public void Reset()
    {
        this.Position = -1;
    }

    public void Dispose() { }
}

// Standards have List as derived Collection...
public class ConcurrentList<T> : ConcurrentCollection<T> { }

This code is still not fully safe, for instance the Count example may still crash, but it allows for iteration, adding and removing across threads. If you want to expose the mutex, do so, then lock around it for your other code constructs like count and contains.

But it's still a bad idea.

Edit: Example usage.

ConcurrentList<string> list = new ConcurrentList<string>();

list.Add("hello");
list.Add("world");
list.Add("foo");
list.Add("bar");

foreach (string word in list)
{
    if (word == "world")
    {
        list.Remove("bar"); // Will not crash the foreach!
    }

    Console.WriteLine(word);
}

Output:

hello
world
foo

Upvotes: 2

Reed Copsey
Reed Copsey

Reputation: 564413

You should wrap this in a class that handles the locking for you, or use a thread-safe collection, such as ConcurrentQueue<T> or one of the other collections in System.Collections.Concurrent.

Exposing the synchronization object to a public API is dangerous, and not a good practice.

Upvotes: 4

Richard Schneider
Richard Schneider

Reputation: 35477

You should always use a lock when accessing the list on different threads.

 public class Sample
 {
     object synch = new object();
     List<Something> list = new List<Something>();

     void Add(Something something)
     {
        lock (synch) { list.Add(something); }
     }

     // Add the methods for update and delete.
  }

Upvotes: 5

Related Questions