C# Why is my lock statement hanging?

Question

I have followed some tutorials on how to create custem event accessors. This is the code I have:

event ControlNameChangeHandler IProcessBlock.OnControlNameChanged
{
    add
    {
        lock (ControlNameChanged)
        {
            ControlNameChanged += value;
        }
    }
    remove
    {
        lock (ControlNameChanged)
        {
            ControlNameChanged -= value;
        }
    }
}

At the moment the code reaches lock(ControlNameChanged) in the add statament, nothing happens. The code doesn't run any further. However my application is still working. It doesn't freeze or something.

What goes wrong?

Answer 1

Somehow, someone else holds a lock. You shouldn't use multicast delegate instances or events for locking, and you shouldn't use public members either, because you cannot control who is locking and when.

Therefore I'd use a separate locking object like this:

private readonly object controlNameChangedSync = new object();

event ControlNameChangeHandler IProcessBlock.OnControlNameChanged
{
  add
  {
    lock (controlNameChangedSync)
    {
      ControlNameChanged += value;
    }
  }
  remove
  {
    lock (controlNameChangedSync)
    {
      ControlNameChanged -= value;
    }
  }
}

Note: the reference to the event changes when doing a += or -= on the delegate.

Answer 2

The += and -= operators change the delegate. So your Add and Remove methods are locking on different objects each time, and you have no synchronization at all.

Now this wouldn't explain blocking but your question is not very clear on what actually happens. I would expect the program to execute 'normally' with the possibility of race-conditions.

Answer 3

Your code is equivalent to

event ControlNameChangeHandler IProcessBlock.OnControlNameChanged {
    add {
        try {
            Monitor.Enter(ControlNameChanged);
            ControlNameChanged = ControlNameChanged + value;
        }
        finally {
            Monitor.Exit(ControlNameChanged);
        }
    } 
    remove { 
        try {
            Monitor.Enter(ControlNameChanged);
            ControlNameChanged = ControlNameChanged - value;
        }
        finally {
            Monitor.Exit(ControlNameChanged);
        }
    } 
}

Note that the object you Exit is different from the one you enter. This means you have one lock taken which is never released, and one lock is released but never taken. You should change your code into this:

private object padlock = new object();
event ControlNameChangeHandler IProcessBlock.OnControlNameChanged {
    add {
        lock (padlock) {
            ControlNameChanged += value;
        }
    } 
    remove { 
        lock (padlock) {
            ControlNameChanged -= value;
        }
    } 
}