C# Multithreading issue with Monitor-class - possible lifelock?

Question

I have a bit of code, which I can't figure out properly. The problem is that the program is multithreaded and within there is a bit of code that should be synchronized so I wrote this:

lock (lockObject)
{
   if (!Monitor.TryEnter(lockObject))
     Monitor.Wait(lockObject);

   //do stuff...
   Monitor.PulseAll(lockObject);
}
Monitor.Exit(lockObject);

the problem I've got is that in some point in time all Threads seem to be sleeping - can someone tell why? The program keeps running along endlessly consuming nearly no cpu but no work is done - when tracing the program I found out that on some point no thread is active but a whole lot of them is sleeping. I know the error mostly (in case of a developer - always) sits 0.5m in front of the monitor - but I cannot figure it out myself... maybe in a few minutes ;)

can someone please explain that to me - thanks in advance.

Answer 1

I'm assming the first lock statement is a typo and you meant lock(lockObject) (lowercase).

I think you're misunderstanding locks a bit here. The if block in your code won't ever be true. The reason why is that lock(lockObject) actually exapands to the following

Monitor.Enter(lockObject);
try {
...
} finally{ 
Monitor.Exit(lockObject);

So by the time you hit the if block you already own the lock and TryEnter should always succeed.

Answer 2

Is there a difference between LockObject and lockObject? It isn't clear...

However! If they are different objects, then first: you can't Wait on a lock that you don't have... and TryEnter will only return false if you specify a timeout. What exactly is that code trying to do?

Without more context, it isn't entirely clear what the PulseAll and Wait are designed to do; for example, here they are used to block the queue when it is too full (Wait), or release it when space becomes available (PulseAll), etc. It is hard to debug threading code without the full interactions between threads.

It sounds like you might just need:

lock (lockObject)
{
    // do stuff
}

---

There are two immediate problems I can see; first, it isn't obvious that you always release the locks you take (i.e. exceptions). Try to just use lock for the Enter/Exit - it'll get it right.

Second; if all the threads call Wait... who is going to wake them up? What are they waiting for? As presented: yes, they will all sleep indefinitely.

Answer 3

That is a strange setup. Is 'LockObject' the same as 'lockObject'? Or is that a typo? If they are the same, then your setup is redundant, as there is no need to call Monitor.TryEnter on something you are already locking. If 'LockObject' is a different object, then why not move the Monitor.Exit to inside the lock statement?