Synchronization mechanism for an observable object

Question

Let's imagine we have to synchronize read/write access to shared resources. Multiple threads will access that resource both in read and writing (most of times for reading, sometimes for writing). Let's assume also that each write will always trigger a read operation (object is observable).

For this example I'll imagine a class like this (forgive syntax and style, it's just for illustration purposes):

class Container {
    public ObservableCollection<Operand> Operands;
    public ObservableCollection<Result> Results;
}

I'm tempted to use a ReadWriterLockSlim for this purpose moreover I'd put it at Container level (imagine object is not so simple and one read/write operation may involve multiple objects):

public ReadWriterLockSlim Lock;

Implementation of Operand and Result has no meaning for this example. Now let's imagine some code that observes Operands and will produce a result to put in Results:

void AddNewOperand(Operand operand) {
    try {
        _container.Lock.EnterWriteLock();
        _container.Operands.Add(operand);
    }
    finally {
        _container.ExitReadLock();
    }
}

Our hypotetical observer will do something similar but to consume a new element and it'll lock with EnterReadLock() to get operands and then EnterWriteLock() to add result (let me omit code for this). This will produce an exception because of recursion but if I set LockRecursionPolicy.SupportsRecursion then I'll just open my code to dead-locks (from MSDN):

By default, new instances of ReaderWriterLockSlim are created with the LockRecursionPolicy.NoRecursion flag and do not allow recursion. This default policy is recommended for all new development, because recursion introduces unnecessary complications and makes your code more prone to deadlocks.

I repeat relevant part for clarity:

Recursion [...] makes your code more prone to deadlocks.

If I'm not wrong with LockRecursionPolicy.SupportsRecursion if from same thread I ask a, let's say, read lock then someone else asks for a write lock then I'll have a dead-lock then what MSDN says makes sense. Moreover recursion will degrade performance too in a measurable way (and it's not what I want if I'm using ReadWriterLockSlim instead of ReadWriterLock or Monitor).

Question(s)

Finally my questions are (please note I'm not searching for a discussion about general synchronization mechanisms, I would know what's wrong for this producer/observable/observer scenario):

• What's better in this situation? To avoid ReadWriterLockSlim in favor of Monitor (even if in real world code reads will be much more than writes)?
• Give up with such coarse synchronization? This may even yield better performance but it'll make code much more complicated (of course not in this example but in real world).
• Should I just make notifications (from observed collection) asynchronous?
• Something else I can't see?

I know that there is not a best synchronization mechanism so tool we use must be right one for our case but I wonder if there are some best practice or I just ignore something very important between threads and observers (imagine to use Microsoft Reactive Extensions but question is general, not tied to that framework).

Possible solutions?

What I would try is to make events (somehow) deferred:

1st solution
Each change won't fire any CollectionChanged event, it's kept in a queue. When provider (object that push data) has finished it'll manually force the queue to be flushed (raising each event in sequence). This may be done in another thread or even in the caller thread (but outside the lock).

It may works but it'll make everything less "automatic" (each change notification must be manually triggered by producer itself, more code to write, more bugs all around).

2nd solution
Another solution may be to provide a reference to our lock to the observable collection. If I wrap ReadWriterLockSlim in a custom object (useful to hide it in a easy to use IDisposable object) I may add a ManualResetEvent to notify that all locks has been released in this way collection itself may rise events (again in the same thread or in another thread).

3rd solution
Another idea could be to just make events asynchronous. If event handler will need a lock then it'll be stopped to wait it's time frame. For this I worry about the big thread amount that may be used (especially if from thread pool).

Honestly I don't know if any of these is applicable in real world application (personally - from users point of view - I prefer second one but it implies custom collection for everything and it makes collection aware of threading and I would avoid it, if possible). I wouldn't like to make code more complicated than necessary.

Answer 1

This sounds like quite the multi-threading pickle. It's quite challenging to work with recursion in this chain-of-events pattern, whilst still avoiding deadlocks. You might want to consider designing around the problem entirely.

For example, you could make the addition of an operand asynchronous to the raising of the event:

private readonly BlockingCollection<Operand> _additions
    = new BlockingCollection<Operand>();

public void AddNewOperand(Operand operand)
{
    _additions.Add(operand);
}

And then have the actual addition happen in a background thread:

private void ProcessAdditions()
{
    foreach(var operand in _additions.GetConsumingEnumerable())
    {
        _container.Lock.EnterWriteLock();
        _container.Operands.Add(operand);
        _container.Lock.ExitWriteLock();
    }
}

public void Initialize()
{
    var pump = new Thread(ProcessAdditions)
    {
        Name = "Operand Additions Pump"
    };
    pump.Start();
}

This separation sacrifices some consistency - code running after the add method won't actually know when the add has actually happened and maybe that's a problem for your code. If so, this could be re-written to subscribe to the observation and use a Task to signal when the add completes:

public Task AddNewOperandAsync(Operand operand)
{
    var tcs = new TaskCompletionSource<byte>();

    // Compose an event handler for the completion of this task
    NotifyCollectionChangedEventHandler onChanged = null;
    onChanged = (sender, e) =>
    {
        // Is this the event for the operand we have added?
        if (e.NewItems.Contains(operand))
        {
            // Complete the task.
            tcs.SetCompleted(0);

            // Remove the event-handler.
            _container.Operands.CollectionChanged -= onChanged;
        }
    }

    // Hook in the handler.
    _container.Operands.CollectionChanged += onChanged;

    // Perform the addition.
    _additions.Add(operand);

    // Return the task to be awaited.
    return tcs.Task;
}

The event-handler logic is raised on the background thread pumping the add messages, so there is no possibility of it blocking your foreground threads. If you await the add on the message-pump for the window, the synchronization context is smart enough to schedule the continuation on the message-pump thread as well.

Whether you go down the Task route or not, this strategy means that you can safely add more operands from an observable event without re-entering any locks.

Answer 2

I'm not sure if this is exactly the same issue but when dealing with relatively small amounts of data (2k-3k entries), I have used the below code to facilitate cross thread read/write access to collections bound to UI. This code originally found here.

public class BaseObservableCollection<T> : ObservableCollection<T>
{
  // Constructors
  public BaseObservableCollection() : base() { }
  public BaseObservableCollection(IEnumerable<T> items) : base(items) { }
  public BaseObservableCollection(List<T> items) : base(items) { }

  // Evnet
  public override event NotifyCollectionChangedEventHandler CollectionChanged;

  // Event Handler
  protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
  {
    // Be nice - use BlockReentrancy like MSDN said
    using (BlockReentrancy())
    {
      if (CollectionChanged != null)
      {
        // Walk thru invocation list
        foreach (NotifyCollectionChangedEventHandler handler in CollectionChanged.GetInvocationList())
        {
          DispatcherObject dispatcherObject = handler.Target as DispatcherObject;

          // If the subscriber is a DispatcherObject and different thread
          if (dispatcherObject != null && dispatcherObject.CheckAccess() == false)
          {
            // Invoke handler in the target dispatcher's thread
            dispatcherObject.Dispatcher.Invoke(DispatcherPriority.DataBind, handler, this, e);
          }
          else
          {
            // Execute handler as is
            handler(this, e);
          }
        }
      }
    }
  }
}

I have also used the code below (which inherits from the above code) to support raising the CollectionChanged event when items inside the collection raise the PropertyChanged.

public class BaseViewableCollection<T> : BaseObservableCollection<T>
  where T : INotifyPropertyChanged
{
  // Constructors
  public BaseViewableCollection() : base() { }
  public BaseViewableCollection(IEnumerable<T> items) : base(items) { }
  public BaseViewableCollection(List<T> items) : base(items) { }

  // Event Handlers
  private void ItemPropertyChanged(object sender, PropertyChangedEventArgs e)
  {
    var arg = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, sender, sender);
    base.OnCollectionChanged(arg);
  }

  protected override void ClearItems()
  {
    foreach (T item in Items) { if (item != null) { item.PropertyChanged -= ItemPropertyChanged; } }
    base.ClearItems();
  }

  protected override void InsertItem(int index, T item)
  {
    if (item != null) { item.PropertyChanged += ItemPropertyChanged; }
    base.InsertItem(index, item);
  }

  protected override void RemoveItem(int index)
  {
    if (Items[index] != null) { Items[index].PropertyChanged -= ItemPropertyChanged; }
    base.RemoveItem(index);
  }

  protected override void SetItem(int index, T item)
  {
    if (item != null) { item.PropertyChanged += ItemPropertyChanged; }
    base.SetItem(index, item);
  }
}