I have usually heard that it is a good idea to unlock any locks before calling event listeners to avoid deadlock. However, since the lock {} block is reentrant by the same thread in C#, is it OK to call events from a locked block or do I need to make a copy of the relevant state data and invoke the event outside the lock block?
If not, please give an example of when it would be a problem to call an event from within a lock {} block.
Thanks
Yes it would not be considered good design to invoke an event from within a held lock. Mostly because there is a transition out of the locked section and if that were to throw an error or have a problem the lock would not necessarily be released.
In general you want to minimize the time you are in the lock to prevent holding the lock too long (causing delays) and also to reduce the chance that you will have code that can fail while the lock is held.
The problem isn't that the event handler might try to call into the lock you already have, the problem is that the event handler might try to acquire some other lock (possibly blocking and setting up for a deadlock), or that the event handler might start some long-running task like a database query (leaving your lock inaccessible to other threads until it finishes). The general rule is that you should hold a lock for as short a time as possible.
Mixing threading with event handlers that you don't control definitely opens you up for trouble. I'm currently having some trouble because I raised an event from the serial port's receiving thread. Some event handler code decided to block and wait until another message is received from the serial port. It's going to be a long wait, because you just blocked the one and only receiving thread! I can't even get mad at anyone because I wrote both pieces of code (a year apart so I had time to forget the details).
I don't recall ever having a need to raise an event from within a lock statement. But it's not hard to imagine things going badly.
When you raise an event, you defer execution to code that may not be your own. This is especially true if you are writing some library or framework, for example, that will be used by others. Inside the event handler, you have absolutely no control over what happens. The event handler could launch a brand new thread and wait for that thread to finish (i.e., Join()) before returning. If that new thread called some function that locked on the same variable as your lock, bingo. Deadlock.
But beyond that, the best practice is to minimize the amount of time you spend inside a lock in order to reduce the "choke point" aspect of the locking. If you raise an event inside the lock, all bets are off.