Most efficient design pattern for event handling with locking

Question

I have an application that receives events asynchronously from an API and can call methods synchronously on this API.

For thread-safety purposes, I need each synchronous function and each event handler in my application to be locked.

However, calling an API method synchronously may lead the API to raise events on a different thread and wait for them to be processed before returning.

This could therefore result in a deadlock as the API would wait for an event to be processed to continue, but in my class the synchronization object would be hit by two different threads and the program would hang.

My current idea is, instead of locking event handlers, to try locking and if not possible (e.g. if the event results from a synchronous call on another thread) to buffer the event in a queue/message pump.

Right before releasing the lock on the synchronous function call, I would then call a ProcessPendingEvents() function so that events could be processed without deadlock.

Do you have any design pattern in mind you would recommend for this kind of situation? I am open to anything.

Here is a simple example to illustrate my current tentative implementation. I really aim at having a class that would behave in a single-threaded way as much as possible:

class APIAdapter {
    readonly object AdapterLock = new object();
    private readonly ConcurrentQueue<Tuple<object, EventArgs>> PendingEvents = new ConcurrentQueue<Tuple<object, EventArgs>>();
    ExternalAPI API = new ExternalAPI();

    APIAdapter() {
        ExternalAPI.Data += ExternalAPI_Data;
    }

    public void RequestData() {
        lock (this.AdapterLock) {
            this.ExternalAPI.SynchronousDataRequest(); //Will cause the API to raise the Data event would therefore deadlock if I had a simple lock() in ExternalAPI_Data.
            this.ProcessPendingEvents();
        }
    }

    private void ExternalAPI_Data(object sender, EventArgs e) {
        if (!Monitor.TryEnter(this.AdapterLock)) {
            this.PendingEvents.Enqueue(Tuple.Create(sender, e));
            return;
        }
        Console.Write("Received event.");
        Monitor.Exit(this.AdapterLock);
    }

    private void ProcessPendingEvents() {
        Tuple<object, EventArgs> ev;
        while (this.PendingEvents.TryDequeue(out ev)) {
            ExternalAPI_Data(ev.Item1, ev.Item2);
        }
    }
}

Look at the `ConcurrentQueue` class, this can be the mediator between the event streams in this case. Alternatively, [Rx](http://msdn.microsoft.com/en-us/data/gg577609.aspx) might be able to help here as it can take events as sources. — Adam Houldsworth, Nov 26 '13 at 17:03
This is what I am using with my current implementation. Do I have a good design pattern or is there something completely different I should think of? — Erwin Mayer, Nov 26 '13 at 17:12
Looks pretty good to me. (Don't like some of the naming conventions, but that's not really the point of the question...) — Will Dean, Nov 26 '13 at 17:13
Which naming convention don't you like in particular? I am always happy to improve them. — Erwin Mayer, Nov 26 '13 at 17:26
The convention in C# is for fields to start with a lower-case letter, to distinguish them from types, methods, and constants. Also, initialisms of more than two characters are treated as words. So it "should" be `ExternalApi api = new ExternalApi();`. But like Will said, that's not really important. — , Nov 26 '13 at 20:20

score 0 · Answer 1 · answered Nov 27 '13 at 08:21

My initial solution was not satisfying: after ProcessPendingEvents() was completed, but before the lock was released, other events could be bufferred and not raised until the next call to ProcessPendingEvents().

Events could also be bufferred at any time if the API sent back two events on different threads, and I was crucially lacking a way to consume these events as soon as the lock were released.

I ended up implementing of much cleaner producer/consumer pattern to control when API events are to be processed, using BlockingCollection. Below is the corresponding code for those who are interested:

class APIAdapter {
    readonly object AdapterLock = new object();
    private readonly BlockingCollection<Tuple<object, EventArgs>> PendingEvents = new BlockingCollection<Tuple<object, EventArgs>>();
    ExternalAPI API = new ExternalAPI();

    APIAdapter() {
        ExternalAPI.Data += ExternalAPI_Data;
        Task.Factory.StartNew(Consume, TaskCreationOptions.LongRunning);
    }

    public void Consume() {
        foreach (var e in this.PendingEvents.GetConsumingEnumerable()) {
            if (this.PendingEvents.IsAddingCompleted) return;
            ProcessData(e.Item1, e.Item2);
        }
    }

    public void RequestData() {
        lock (this.AdapterLock) {
            this.ExternalAPI.SynchronousDataRequest(); 
        }
    }

    private void ExternalAPI_Data(object sender, EventArgs e) {
        this.PendingEvents.Add(Tuple.Create(sender, e));
    }

    private void ProcessData(object sender, EventArgs e) {
        lock (this.AdapterLock) {
            Console.Write("Received event.");
        }
    }
}

Most efficient design pattern for event handling with locking

1 Answers1