Threadsafe oneshot-event which fires on subscription if the event was fired in the past

Question

Assumptions

Basically, a Connection class has a "Disconnect" event. Subscribing to this event isn't thread-safe, because the disconnection may fire from another thread right before I subscribe. So checking before the subscription doesn't help.

Checking for the disconnect after subscription doesn't help either because the event may have fired in the meanwhile (2 threads might execute the same "observer" twice).

(My) Solution:

An event always fired once (and only once), even if the event itself already happened before Source is on GitHub as well.

Questions:

Are there other simpler solutions addressing this? (by simpler I mean from an outside or usage perspective)

Do you see any race conditions or things that could go wrong?

Maybe you have optimizations or simplifications to add?

Input is highly appreciated!

/// <summary>
/// Triggers if the event is invoked or was invoked before subscribing to it.
/// <para> Can be accessed safely by multiple threads.</para>
/// </summary>
public class AutoInvokeEvent<Sender, Args>
{
    public delegate void EventHandle(Sender sender, Args arguments);

    /// <summary>
    /// Handle will be invoked if the event was triggered in the past.
    /// <para>Unsubscribing happens automatically after the invocation and is redundant if done from the event handle.</para>
    /// </summary>
    public event EventHandle Event
    {
        add
        {
            if (!Subscribe(value))
                value(m_sender, m_eventArgs);
        }
        remove { InternalEvent -= value; }
    }

    private event EventHandle InternalEvent;

    // this is my personal lock implementation. in this case it is used like any other lock(object) so just ignore it
    private SafeExecutor m_lock = new SingleThreadExecutor();
    private volatile bool m_invoked = false;

    Sender m_sender;
    Args m_eventArgs;

    /// <summary>
    /// Invokes all subscribed handles with the given parameters. 
    /// <para>All calls after the first are ignored.</para>
    /// </summary>
    public void Invoke(Sender sender, Args args)
    {
        GetEventHandle(sender, args)?.Invoke(m_sender, m_eventArgs);
    }

    private EventHandle GetEventHandle(Sender sender, Args args)
    {
        return m_lock.Execute(() =>
        {
            if (m_invoked)
                return null;

            m_sender = sender;
            m_eventArgs = args;
            m_invoked = true;
            EventHandle handle = InternalEvent;
            InternalEvent = null;
            return handle;
        });
    }

    /// <returns>Returns true if subscription was successful and false if handle needs to be invoked immediately.</returns>
    private bool Subscribe(EventHandle handle)
    {
        return m_lock.Execute(() =>
        {
            if (!m_invoked)
                InternalEvent += handle;

            return !m_invoked;
        });
    }
}

How the class could be used :

class Connection
{
    public AutoInvokeEvent<object, EndPoint> OnDisconnect = new AutoInvokeEvent<object, EndPoint>();

    public Disconnect()
    {
        OnDisconnect.Invoke(this, endpoint);
    }
}

void main()
{
    Connection connection = new Connection();
    connection.OnDisconnect.Event += DoStuffOnDisconnect;
}

void DoStuffOnDisconnect(object sender, EndPoint endpoint) { }

Answer 1

Thread safety

Both Subscribe and GetEventHandle use lock and and+remove on field event are thread-safe too. You can even remove the volatile from m_invoked because you only ever access it under the lock from those two methods.

API and naming

Event and Invoke are pretty obvious, especially with the description. I would only change the signature of Invoke to return bool (true = invoked, false = not invoked, already done before) to better reflect the fact, that it won't throw exception if you try to invoke it multiple times (could as well be named TryInvoke, maybe changing Invoke to throw exceptions if invoked multiple-times).

GetEventHandle does not sound as good name to me, because it does much more than just getting the handle, it stores sender+args, takes current handle and clears it. I would either use it (and Subscribe) directly where they are used (only one place for each), which also solves naming-problem, or make them local functions.

AutoInvokeEvent hmm, how about OneShotEvent? Although it automatically calls subscribed handlers even after the event was fired, this is more about how it does it under the hood, than what the API is about - it will execute the handler exactly once when it is fired or if it already was fired.

Optimization

I don't think any optimization is necessary. You would have to first specify (and or test/profile) how often is the event subscribed compared to invoking it, to even have some direction in which to optimise. I can see you have switched to using some read-write-lock in the implementation you currently have on GitHub, makes sense (concurrent subscriptions, if that is trully important to you). Could as well be implemented by double-checked-locking, thanks to the fact this is one-shot event, but I don't think it is worth it, too easy to make a mistake, very hard to make it right. (The idea is to have volatile object m_lock and make it null when you fire the event, instead of using that bool m_invoked.)