Adding and Removing event handlers for multithreaded applications

Question

I have a question how to properly add/remove event handler in multithreaded environment for asynchronous callbacks.

I have MyCore class which receives asynchronous callbacks from ProxyDLL which dispatches callback from unmanaged code. I have form(managed) which subscribes to events.

What would be the right approach to attaching/detaching from event. I noticed that MulticastDelegate has _invocationcount. What it does? Does inner logic of the event blocks detaching from event if callback call is in process until callback is done? Is _invocationcount exists for that puprose? Is detathing from event (in general) is treadsafe?

class Form1
{
  EventHandler m_OnResponse;
  Int32 m_SomeValue;
  Form1()
  {
    m_OnResponse = new EventHandler(OnResponseImpl);
    m_MyCore.SetCallBackOnLogOn(m_OnResponse);
  }
  ~Form1()
  {
    m_MyCore.ReleaseCallBackOnLogOn(m_OnResponse);
  }
  private OnResponseImpl(object sender, EventArgs e)
  {
    Thread.Sleep(60*1000);

    m_SomeValue = 1;             // <<-- How to/Who guarantees that Form1 obj is still
                                 // alive. May be callback was invoked earlier and
                                 // we just slept too long

    if (!this.IsDisposed)
    {
        invokeOnFormThread(DoOnResponseImpl, sender, e);
    }
  }
}

class MyCore
{
  private event EventHandler OnLogOn;
  public void SetCallBackOnLogOn(EventHandler fn)
  {
    // lock (OnLogOn)
    {
        OnLogOn += fn;
    }
  }
  ReleaseCallBackOnLogOn(EventHandler fn)
  {
    // lock (OnLogOn)
    {
        OnLogOn -= fn;
    }
  }
  public void DoDispatchOnLogOn()
  {
    // lock (OnLogOn)
    {
      if (OnLogOn != null)
      {
        OnLogOn(this, null);
      }
    }
  }
}

score 3 · Accepted Answer · edited May 23 '17 at 12:27

Default event add and remove operations are already thread-safe. You do not need worry about that part in most cases. It is the invocation of the multicast delegate that you need to worry about.

public void DoDispatchOnLogOn()
{
  EventHander local;
  lock (this)
  {
    local = OnLogOn;
  }
  if (local != null)
  {
    local(this, null);
  }
}

What I did here was to create a local variable that will hold the OnLogOn delegate chain. I am exploiting the immutability of multicast delegates here so that we can do a thread-safe check for null and the invoke sequence. The lock is only used to ensure a "fresh" read of OnLogon and it is strictly optional if you do not mind gettting a "stale" read.

Update:

I need to be sure that callback is not invoked when ReleaseCallBackOnLogOn ended unsubscribing delegate.

For the most part the code I have will not attempt to execute any event handlers that have been unsubscribed. There is a slight a race between removing an event handler and raising the event which could cause the event handler to get executed even though it was just recently removed.

I need to be sure that my class instance is still alive, until call to callback is fully done.

Delegates hold a reference to the class instance containing the target method. This will keep your instance rooted and thus not eligible for garbage collection. You do not need to worry about this.

I should point out that the ~Form1 finalizer is not a good a place to remove event handlers. Remember, delegates hold a reference to the instance containing the target method so in most cases that finalizer will not get called and the event handler will not be removed from the event.

Thanks! It's a good point. And regarding safety - do you want to say that unsubscribing listener from event is a blocking call? — adspx5, Apr 17 '12 at 18:52
@adspx5: C# 4.0 and higher uses `Interlocked.CompareExchange` to do the subscribing and unsubscribing. C# 3.0 and lower uses `lock(this)`. So C# 3.0 and lower will block if the lock is currently held by someone else. — Brian Gideon, Apr 17 '12 at 19:39
It is not quite what I'm trying to understand. I understood, but don't worry about invocation list changes. I worry about lifetime of my delegate. I need to be sure that callback is not invoked when ReleaseCallBackOnLogOn ended unsubscribing delegate. Guess that my delegate Sleeps for 10 sec and changing MyForm::m_Somevalue after that. I need to be sure that my class instance is still alive, until call to callback is fully done. — adspx5, Apr 17 '12 at 20:47

Adding and Removing event handlers for multithreaded applications

1 Answers1