Thread-safe events -- locking on privately held references

Question

I was reading about delegates and events on this webpage to get a better understanding of them and how they relate to each other. While reading, I came across this statement:

That goes against the principle of locking on privately held references to avoid accidental deadlocks.

The author was referring to a lock with a reference to this object in the add and remove operations of an event.

Further details weren't provided about the principle he was referring to, maybe because it's supposed to be obvious. In any case, it wasn't obvious to me. Can someone enlighten me on this and provide some more detail?

Answer 1

Well, if you read a few lines further, it seems that certain methods perform a locking operation, but you don't know which object is used for the locking. And that is a very different problem, because you can't lock on the same object if that is what you want to do. All code trying to protect the same data must obviously use the same lock.

As far as the deadlock concerns, yes, that could be a problem. Thread 1 locks on object A, then on object B. Thread 2 locks on object B, then performs an innocent looking operation. That should be fine, right? But if the innocent looking operation locks on object A, you have a deadlock.

What does "privately held" to do with anything? Thread 1 and Thread 2 can only deadlock if the lock the same variable. If "harmless looking operation" locks a variable that is privately held and not accessible to other code, then you won't get into a deadlock.

Answer 2

First - deadlocks. This is when you have one thread inside a lock, trying to lock something else, and that something else is locked by another thread waiting for the lock that the original thread had. They just sit there waiting for the other thread to finish, but neither can. Bad times.

The issue is that each instance of an object can serve as the context of a lock. By locking on private instances, you're (reasonably) assured that only code in that class can lock on that instance, so you can limit your analysis to see if that code can deadlock.

By locking on this, code outside of the class could also lock on that instance, making it much more difficult to figure out potential deadlock scenarios in your codebase, and impossible once you consider external consumers of your code.

That said, I find this best practice to be a bit of a cargo cult. Manual locking of variables is decreasingly common, and very often the need to lock is around a class scope. Making a new object just to lock on it is unintuitive and very often overkill.

Still, it is very important to understand why this sort of thing is done for those times when it is not overkill.