Thread safe class

The following class's method public IAsyncResult BeginInvoke(Delegate method, object[] args) is called by a third party dll. This dll may or may not be multi-threaded.

The dll also calls public bool InvokeRequired { get; } to check if it should call the BeginInvoke method. The third party dll does this every time it gets a socket message from a server.

If the InvokeRequired is false, instead of calling BeginInvoke the dll publishes an event of the message to any method that is subscribed.

I want to use the MessageSynchronizer class (below) in an ASP.NET application.
The class will be created with a new instance on every request.

I am fairly new to multithreading, and I would like to know if the way I have designed this class is both thread safe and efficient.

An instance would be created like so:

var synchronizer = new MessageSynchronizer((x) => Console.WriteLine(x), MessageSynchronizer.CallbackOptions.Handle | MessageSynchronizer.CallbackOptions.Async);

Then the synchronizer instance gets passed to the third parties dll's constructor.
Is the below class adequate?

```
using System;
using System.ComponentModel;
using System.Threading.Tasks;

namespace ConsoleApplication1
{
class MessageSynchronizer : ISynchronizeInvoke
{
private readonly object[] _locks = null;
private readonly bool _isLockFree = false;

///
/// For manual handling of softdial messages.
///
public Action Callback { get { return Get(() => _callback, 0); } set { Set(x => _callback = x, value, 0); } }
private Action _callback = null;

public CallbackOptions Options { get { return Get(() => _options, 1); } set { Set(x => _options = x, value, 1); } }
private CallbackOptions _options = CallbackOptions.None;

private T Get(Func field, int locker)
{
if (_isLockFree)
throw new Exception("Cannot get properties as CallbackOptions is set to Never.");

I'm not entirely sure what exactly you intend to be thread-safe. Right now just the access to the Callback and Options properties is locked and those locks are useless because reference assignment in C# is guaranteed to be atomic.

If you assume that your locking will achieve that the call Callback(message); is mutually exclusive then you are mistaken.

So you can replace this code block:

/// 
    /// For manual handling of softdial messages.
    /// 
    public Action Callback { get { return Get(() => _callback, 0); } set { Set(x => _callback = x, value, 0); } }
    private Action _callback = null;

    public CallbackOptions Options { get { return Get(() => _options, 1); } set { Set(x => _options = x, value, 1); } }
    private CallbackOptions _options = CallbackOptions.None;

    private T Get(Func field, int locker)
    {
        if (_isLockFree)
            throw new Exception("Cannot get properties as CallbackOptions is set to Never.");
        lock (_locks[locker])
            return field();
    }

    private void Set(Action field, T value, int locker)
    {
        if (_isLockFree)
            throw new Exception("Cannot set properties as CallbackOptions is set to Never.");
        lock (_locks[locker])
            field(value);
    }

with

public Action Callback { get; set; }
    public CallbackOptions Options { get; set; }

and it will behave the same as before.

If your goal is that in the case of two threads calling BeginInvoke at the same time the callbacks are executed mutually exclusive then you need to wrap the call in a lock:

private readonly object _callbackLock = new object();

 ...

 lock (_callbackLock) { Callback(message); }

Update

In BeginInvoke you make a local copy callback = Callback but when invoking the action you use the property again rather than the copy - you should fix that.

StackExchange Code Review Q#38956, answer score: 2