CODEWITHSUNDEEP
wpfsilverlightprismeventaggregator
pfaz
pfaz

Reputation: 123

EventAggregator, is it thread-safe?

Is this thread-safe?

The EventAggregator in Prism is a very simple class with only one method. I was surprised when I noticed that there was no lock around the null check and creation of a new type to add to the private _events collection. If two threads called GetEvent simultaneously for the same type (before it exists in _events) it looks like this would result in two entries in the collection.

    /// <summary>
    /// Gets the single instance of the event managed by this EventAggregator. Multiple calls to this method with the same <typeparamref name="TEventType"/> returns the same event instance.
    /// </summary>
    /// <typeparam name="TEventType">The type of event to get. This must inherit from <see cref="EventBase"/>.</typeparam>
    /// <returns>A singleton instance of an event object of type <typeparamref name="TEventType"/>.</returns>
    public TEventType GetEvent<TEventType>() where TEventType : EventBase
    {
        TEventType eventInstance = _events.FirstOrDefault(evt => evt.GetType() == typeof(TEventType)) as TEventType;
        if (eventInstance == null)
        {
            eventInstance = Activator.CreateInstance<TEventType>();
            _events.Add(eventInstance);
        }
        return eventInstance;
    }

Upvotes: 6

Views: 2860

Answers (3)

Peter Gfader
Peter Gfader

Reputation: 7741

No, not thread safe.

  1. The instance member access of the List class itself are NOT thread safe, as per MSDN under Thread safety
  2. The method itself is not thread safe
    1. 2 threads could enter the method at the same time
    2. Both try to get the FirstOrDefault
    3. Both get nothing
    4. Both add a new TEventType

I would

  1. switch to one of the System.CollectionConcurrentX collections in .NET 4
    http://msdn.microsoft.com/en-us/library/system.collections.concurrent.aspx
    or
  2. do your own locking

Upvotes: 4

JerKimball
JerKimball

Reputation: 16894

Well, based on that code paste, I'd say no, it's not 100% thread safe.

Of course, you have the source, so you can just add the lock yourself. :)

Actually, as a general rule of thumb, I include the entire CAL project in my solution, at least at the start. It helps a lot in debugging those odd region registration/creation exceptions...

Upvotes: 0

Peter Gfader
Peter Gfader

Reputation: 7741

It depends what "_events" is...

There are some awesome new thread-safe classes in .NET 4 ...
Check out http://msdn.microsoft.com/en-us/library/system.collections.concurrent.aspx

Upvotes: 0

Related Questions