Thread safe queue (list) in .net

Question

I need to create a thread safe list of items to be added to a lucene index.

Is the following thread safe?

public sealed class IndexQueue
{
    static readonly IndexQueue instance = new IndexQueue();
    private List<string> items = new List<string>();

    private IndexQueue() { }

    public static IndexQueue Instance {
        get { return instance; }
    }

    private object padlock = new object();

    public void AddItem(string item) {
        lock (padlock) {
            items.Add(item);
        }
    }
}

Is it necessary to lock even when getting items from the internal list?

The idea is that we will then have a separate task running to grab the items from indexqueue and add them to the lucene index.

Thanks Ben

Answer 1

Is it necessary to lock even when getting items from the internal list?

The List class is not thread-safe when you make modifications. It's necessary to lock if:

• You use a single instance of the class from multiple threads.
• The contents of the list can change while you are modifying or reading from the list.

Presumably the first is true otherwise you wouldn't be asking the question. The second is clearly true because the Add method modifies the list. So, yes, you need it.

When you add a method to your class that allows you to read back the items it is also necessary to lock, and importantly you must use the same lock object as you did in AddItem.

Answer 2

Your implementation seems thread-safe, although you will need to lock when reading from items as well - you can not safely read if there is a concurrent Add operation. If you ever enumerate, you will need locking around that as well and that will need to live as long as the enumerator.

If you can use .net 4, I'd strongly suggest looking at the System.Collections.Concurrent namespace. It has some well tested and pretty performant collections that are thread-safe and in fact optimized around multiple-thread access.

Answer 3

Yes; while retrieval is not an intrinsically unsafe operation, if you're also writing to the list, then you run the risk of retrieving in the middle of a write.

This is especially true if this will operate like a traditional queue, where a retrieval will actually remove the retrieved value from the list.