CODEWITHSUNDEEP
c#.netlocking
Martijn
Martijn

Reputation: 24809

C# Why is my lock statement hanging?

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?

Upvotes: 4

Views: 2664

Answers (3)

erikkallen
erikkallen

Reputation: 34421

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;
        }
    } 
}

Upvotes: 8

Henk Holterman
Henk Holterman

Reputation: 273681

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.

Upvotes: 4

Lucero
Lucero

Reputation: 60276

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.

Upvotes: 13

Related Questions