Is this thread safe?

[theoobe]

using System;
using System.Collections.Generic;

namespace core.Extensions
{
    class ConcurrentList<T>
    {
        private List<T> Items { get; set; }
        private object Locker { get; set; }

        public ConcurrentList()
        {
            this.Items = new List<T>();
            this.Locker = new object();
        }

        public void Add(T obj) // called from GUI thread
        {
            lock (this.Locker)
                this.Items.Add(obj);
        }

        public T TakeFirst(Predicate<T> predicate) // called from GUI thread
        {
            T obj = default(T);

            lock (this.Locker)
            {
                int index = this.Items.FindIndex(predicate);

                if (index > -1)
                {
                    obj = this.Items[index];
                    this.Items.RemoveAt(index);
                }
            }

            return obj;
        }

        public void ForEach(Action<T> action) // called from server thread
        {
            T[] copy;

            lock (this.Locker)
                copy = this.Items.ToArray();

            foreach (T obj in copy)
                action(obj);
        }
    }
}

Items are added and removed from the main GUI thread. The server's thread will periodically loop through the list items. I would have preferred to have used one of the System.Collections.Concurrent classes but they all seem to lack the ability to remove specific items, and instead always remove from the start or end of the collection, depending on whether it is LIFO or FIFO.

Does my attempt at a thread safe list look to be thread safe? Would there be a better/more efficient way of tackling this problem?

Thanks.

[theoobe]

Nevermind. ConcurrentDictionary seems to be what I need.

[Momerath]

To answer your question anyway, no it's not thread safe. Your ForEach method, if operating on objects, may reference an object that is no longer part of the collection while it is running.

And I would think ConcurrentBag would do what you are doing here. You'd use TryTake instead of RemoveAt.