Are `System.MulticastDelegate`'s thread-safe?

Question

I'm looking for someone that might know more about this, my gut tells me that the answer is "no, it is not thread-safe" but I want to be sure.

In order to illustrate my question, I have provided some context with this class

public class MyContext
{
    private readonly object _lock = new object();
    public delegate bool MyDelegate(MyContext context);
    private MyDelegate _multicastDelegate;

    public MyContext()
    {
        _multicastDelegate = null;
    }

    public void AddDelegate(MyDelegate del)
    {
        lock(_lock)
        {
            _multicastDelegate += del;
        }
    }

    public void RemoveDelegate(MyDelegate del)
    {
        lock(_lock)
        {
            _multicastDelegate += del;
        }
    }

    public void Go()
    {
        _multicastDelegate.Invoke(this);
    }
}

Edit: I added locks to the example above, but it's really not the point of my question.

I am trying to understand better if the array that holds the invocation list is thread-safe. Frankly, I am not clearly seeing how this all gets put together and some help would be appreciated.

According to documentation I have found, the only quote that provides no real insight is the following:

A MulticastDelegate has a linked list of delegates, called an invocation list, consisting of one or more elements. When a multicast delegate is invoked, the delegates in the invocation list are called synchronously in the order in which they appear. If an error occurs during execution of the list then an exception is thrown.

https://msdn.microsoft.com/en-us/library/system.multicastdelegate.aspx

Thanks in advance.

You'd need to define "thread-safe" in this context for the question to have meaning. Note that using `+=` and `-=` doesn't modify the existing delegate instance - it returns a *new* instance. The code you've shown is not thread-safe in that if two threads call (say) `AddDelegate` simultaneously, you could easily end up with one of those effectively being "missed". This is why auto-generated field-like event accessors do more work. — Jon Skeet, Feb 08 '18 at 10:26
I might have rushed a bit too quickly in putting together the code example. Given your comment, my question was intended to be in terms of adding/removing to the invocation list. — Svek, Feb 08 '18 at 10:29
@JonSkeet -- I made a few edits, but I think I need help with my wording... I am assuming that the invocation list is essentially an array. I'd like to know if that array is thread-safe to add/remove items to. — Svek, Feb 08 '18 at 10:38
Delegate objects are *immutable*. Hard to see from the syntax sugar, but the list of delegate targets never gets modified. Only the constructor ever creates that list. So when you call Go() while at the same time another thread is busy calling Add or Remove then nothing can blow up. Other than the added target method not getting called or the removed target still getting called. That's generally a threading race bug that doesn't deserve the "thread-safe" moniker, but it won't surprise the client programmer. Well, shouldn't. — Hans Passant, Feb 08 '18 at 10:58
@HansPassant I think you're touching upon the same issue Jon does in his comment and that's that "thread-safe" is a rather meaningless term because it means different things in different contexts. — Lasse V. Karlsen, Feb 08 '18 at 12:07

score 6 · Accepted Answer · answered Feb 08 '18 at 10:49

Delegates are immutable. You never change a delegate. Any method that appears to mutate a delegate is in fact creating a new instance.

Delegates are immutable; once created, the invocation list of a delegate does not change.

There is thus no cause for concern that the invocation list may be updated whilst a delegate is being invoked.

What you do have to guard against, however, and have failed to do in your method is when the delegate may in fact be null.

(new MyContext()).Go();

will cause an exception. You used to have to guard against this by reading the value into a local variable, testing it for null and then invoking using it. It can now more easily be resolved as:

public void Go()
{
    _multicastDelegate?.Invoke(this);
}

Thank you. I've been up for a while now and I was having trouble getting the wording together, but the assignment operator should have been my "big hint". --- your answer helped bring a lot of clarity thanks again. — Svek, Feb 08 '18 at 10:53

acelent · Answer 2 · 2021-12-16T19:52:38.087

The definition of thread-safe as used by MSDN documentation means code that is properly synchronized. It usually doesn't state on what it synchronizes, but it can be the class object for static members, the instance object for instance members, or it can be some inner object, such as SyncRoot in many collection types.

Although delegates are immutable, you must still synchronize properly. .NET and C#, unlike Java, don't guarantee safe publication, so if you don't guarantee synchronization, you can observe a partially initialized object in other threads¹.

To turn your code thread-safe, you just need to use _lock when reading from the delegate field as well, but you can call Invoke outside the lock, landing onto the delegate the responsibility to keep its own thread safety.

public class MyContext
{
    private readonly object _lock = new object();
    public delegate bool MyDelegate(MyContext context);
    private MyDelegate _delegate;

    public MyContext()
    {
    }

    public void AddDelegate(MyDelegate del)
    {
        lock (_lock)
        {
            _delegate += del;
        }
    }

    public void RemoveDelegate(MyDelegate del)
    {
        lock (_lock)
        {
            // You had a bug here, +=
            _delegate -= del;
        }
    }

    public void Go()
    {
        MyDelegate currentDelegate;
        lock (_lock)
        {
            currentDelegate = _delegate;
        }
        currentDelegate?.Invoke(this);
    }
}

Microsoft's .NET Framework implementation always makes volatile writes (or so they say), which kind of gives you safe publication implicitly, but I personally don't rely on this.

Are `System.MulticastDelegate`'s thread-safe?

2 Answers2