thread safety of a static class

Question

I want to create my own event system in my windows application in c#. To do this, i write the following class:

internal class EventManager
{
    private static List<EventRecord> s_listEvents = new List<EventRecord>();

    public static void AddEvent(EventRecord record)
    {
        record.EventDate = DateTime.Now;
        s_listEvents.Add(record);
    }

    public static List<EventRecord> GetRecordsByDate(DateTime date)
    {
        var r = (from l in s_listEvents
                 where l.EventDate >= date
                 select l).ToList<EventRecord>();
        return r;
    }
}

I want to be sure that EventManager class is thread-safe. Because i am going to create hundreds of threads at the same time in my application. All the threads will most probably use this class to generate events. And GetRecordsByDate function may be called from outside of the class at the time when AddEvent function will be called from different threads.

Simply, can you tell me that this design is appropriate for multi-thread windows application? If this is not thread-safe, then how can i make my class or its members thread-safe? Should i use a sync object to lock the whole EventManager class or should i use readwritelocker to lock my s_listEvents static member?

Answer 1

Instead of using List<T>, you should use ConcurrentBag<T> instead.

ConcurrentBag is a thread-safe bag implementation, optimized for scenarios where the same thread will be both producing and consuming data stored in the bag.

More information:

http://msdn.microsoft.com/en-us/library/dd381779.aspx

Also, be careful to create how many thread to access, over 100 threads would make slow performance because it takes time for switch context.

Edit: For .NET 3.5 you can make thread-safe by using simple lock

internal class EventManager
{
    private static List<EventRecord> s_listEvents = new List<EventRecord>();
    private static object _syncObject = new object();


    public static void AddEvent(EventRecord record)
    {
        record.EventDate = DateTime.Now;
        lock(_syncObject)
        {
           s_listEvents.Add(record); 
        }

    }

    public static List<EventRecord> GetRecordsByDate(DateTime date)
    {
        lock (_syncObject)
        {
             var r = (from l in s_listEvents
                 where l.EventDate >= date
                 select l).ToList<EventRecord>();

             return r;
        }

    }
}

Edit:

Depend on your situation, if you read data very frequently, use ReaderWriterLockSlim with ReaderWriterLock would be better for the whole application because it allow multiple threads to read data.

If not, use lock which has better performance in general.

See the link:

http://blogs.msdn.com/b/pedram/archive/2007/10/07/a-performance-comparison-of-readerwriterlockslim-with-readerwriterlock.aspx