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;        /// <summary>        /// For manual handling of softdial messages.        /// </summary>        public Action<string> Callback { get { return Get(() => _callback, 0); } set { Set(x => _callback = x, value, 0); } }        private Action<string> _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<T>(Func<T> 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<T>(Action<T> field, T value, int locker)        {            if (_isLockFree)                throw new Exception(Cannot set properties as CallbackOptions is set to Never.);            lock (_locks[locker])                field(value);        }        /// <summary>        /// Use the default CampaignManagerClass's event        /// subscriber pattern (recommended).        /// </summary>        public MessageSynchronizer(CallbackOptions options = CallbackOptions.None)        {            _callback = null;            _options = options;            _isLockFree = options.HasFlag(CallbackOptions.Never);            if (!_isLockFree)            {                _locks = new object[]                {                    new object(),                    new object()                };            }        }        /// <summary>        /// Use custom message reading, the CampaignManagerClass's event callbacks        /// will be ignored and your custom message handler will be used instead        /// (with the CallbackOptions.HandleCallback flag set).        /// </summary>        /// <param name=callback>A method that takes the raw softdial socket messages.</param>        /// <param name=options>The callback options when messages are received.</param>        public MessageSynchronizer(Action<string> callback, CallbackOptions options = CallbackOptions.None)            :this()        {            if (callback == null)                throw new ArgumentNullException(callback);            _callback = callback;            _options = options;        }        /// <summary>        /// The protected override MessageReceived(string) method        /// from the RouterNet.RouterConnection (in RouterNet.dll) class calls this method        /// when it gets a message from the Sytel.Mdn2.dll and this.InvokeRequired is true.        /// The RouterConnection is a static field on the        /// CampaignManager.CampaignManagerClass (in RouterNet.dll).        /// </summary>        public IAsyncResult BeginInvoke(Delegate method, object[] args)        {            var message = (string)args[0];  // The args passed from CampaignManagerClass.RouterConnection.MessageReceived is always one, non-null string.            var options = Options;            /* *             * The if statement below is designed in such a way as             * to minimise lock contention. While deadloacks aren't possible,             * locks are only taken when necessary, this can avoid             * thread blocking. Locks are implemented in the properties.             */            if (options.HasFlag(CallbackOptions.Handle))            {                var callback = Callback;                if (callback != null && message != string.Empty)  // The message should never be empty, but just incase.                {                    if (options.HasFlag(CallbackOptions.Async))                    {                        Task.Factory.StartNew(() =>                        {                            Callback(message);                        });                    }                    else                    {                        Callback(message);                    }                }            }            return null;        }        public object EndInvoke(IAsyncResult result)        {            throw new NotImplementedException();        }        public object Invoke(Delegate method, object[] args)        {            throw new NotImplementedException();        }        public bool InvokeRequired        {            get {  return _isLockFree ? false : Options.HasFlag(CallbackOptions.Handle); }        }        [Flags]        public enum CallbackOptions        {            /// <summary>            /// The callback method will be ignored.            /// </summary>            None = 0,            /// <summary>            /// If provided, the callback method will            /// be called with the message from softdial            /// instead of default subscribed event.            /// </summary>            Handle = 1,            /// <summary>            /// If when calling the callback method            /// the call should be done asynchronously,            /// by default it's called synchronously.            /// </summary>            Async = 2,            /// <summary>            /// Any attempt to set the change the callback            /// method or CallbackOptions results in an            /// exception. This option avoid locks            /// altogether, but still remaining thred safe.            /// </summary>            Never = 4        }    }}
Thread safe class
c#;multithreading;thread safety
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:    /// <summary>    /// For manual handling of softdial messages.    /// </summary>    public Action<string> Callback { get { return Get(() => _callback, 0); } set { Set(x => _callback = x, value, 0); } }    private Action<string> _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<T>(Func<T> 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<T>(Action<T> 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<string> 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); }UpdateIn 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.