Help with locks

Archive
1

I’m cleaning up some code in the hopes of incorporating some of the things I’ve learned in the past months and it has become apparent I don’t really understand how locks work.

Most of my apps have to read data from a lot of sensors. Many of these sensors are RS232 and I use an event handler for each sensor’s serial port to grab the bytes as the sensor sends them. Once I get a complete message I put in in “my” queue (that I modified from @ taylorza’s class). In the future other threads may want to use the queue so I’m trying to use locks to make everything thread safe.

My 2 basic questions are 1) does every class in every thread that needs to enqueue something need it’s own lock defined in the class and 2) Where do I put the locks?

In my current implementation, each sensor generally has it’s own class and it’s own lock defined like this:



Is that right or should I use one global lock object or ?.

Then when that class' event handler (or any other method in that class ) needs to enqueue something it does this:


```cs]lock (GPSLock) { Program.sQueue.Enqueue(qS); }[/code


The Enqueue method is pretty simple.  Should I lock it also? Or lock it instead of locking the calls to it?


```cs
       public void Enqueue(qStruct inStruct)
        {
            if (count >= qDepth)
            {
                throw new InvalidOperationException("Queue is full");
            }
            else
            {
                queue[head].qRecordType = inStruct.qRecordType;
                queue[head].timeStamp = inStruct.timeStamp;
                queue[head].state = inStruct.state;
                Array.Copy(inStruct.byteArray, queue[head].byteArray, inStruct.byteArray.Length);
                head = (head + 1) % qDepth;
                count++;
            }
        }

Looking forward to your suggestions.

2

Here is a brief description of locks.

You can use any object for a lock, but many people recommend to create extra objects as locks, for less confusion.

if you have a lock on an object

lock(lockObject)
{
  ...
}

all other threads, that tries to enter a block with the same lock object (or the same lock) are blocked, until the 1st tread leaves the block.
One thread can enter the same lock several times.


void SomeMethod()
{
   lock(lockObject)
   {
      ...
   }
}

...

lock(lockObject)
{
   SomeMethod();
   ...
}

The code above creates no deadlock, because the same thread can enter multiple locks on the same object.

For every resource or part of code, where you want no other thread interfering you use the same lock object.
If you access a queue from multiple threads, you should put a lock around every access, using the same object. In fact you could use the queue object itself.

Where to store locks objects:
Exactly where you need them. It doesn’t matter if it’s static or not, it just must not be null.
Make sure everyone who needs it can access it, but if possible limit the access as good as possible, so no one who should not use it can do so.

Warning:
Extensive use of locks can easily lead to deadlocks! Especially if you use the same lock object, even if it is not really necessary.
Recently I had several dead locks, when I tried to make applications “more stable” by applying some locks.
It’s always a good idea to double check threading code, may be even from an 2nd person.
I had this

You can also read the original MS docs on this.

So far I have seen no difference to NETMF.

Edit:
But what about non blocking lock:
Here is how I usually create a non blocking lock (piece of code that can be executed only once at a time, but does no block when called a 2nd time)

Edit: added a volatile to the _isExecuted field, as suggested by taylorza

class SomeClass
{
  private volatile bool _isExecuted;
  private readonly object _lockObject = new object();

  public void RunOnceAtAtimeMethod()
  {
    if (_isExecuted)
      return;
    lock(_lockObject)
    {
      if(_isExecuted)
       return;
      _IsExecuted = true;
    }
    try
    {
      // code to run only once at a time
    }
    finally
    {
      _isExecuted = false;
    }
  }
}

If I remember right, this is called the double lock pattern.

2 Likes
3

I think this is sinking in. Seems like all I have to do is put a lock around all the code in my Enqueue method and that should handle it.

I don’t think I need to worry about other threads getting blocked since the enqueing operation happens pretty fast relative to all the serial port events.

If someone has an example showing good locking practices they could point me at, that would be a big help also.

Thanks - Gene

4

@ Reinhard Ostermeier - Double checked locking has a few caveats. As a matter of best practice you should consider making the variable being guarded volatile. This will not make a difference in .NETMF, but even with MS strong memory model implementation on x86, with out it double checked locking will potentially not work on full .NET.

I will not bore you with the details (and typing on the iPad is a pain), but suffice to say, using volatile inserts the required memory barrier ensuring that compiler optimizations and instruction reordering paired with the target memory model will not work against you.

As for dead locks, one good rule when using multiple locks, always, always acquire your locks in the same order every time. If you have two lock objects A and B, and somewhere in your code you acquire a lock on B and then acquire a lock on A, make sure that every other place in your code you acquire the lock on both you do it in the same order. This can sneak up on you esp. When calling functions from with in your locks and those functions acquire other locks. You need good rules and coding standards/conventions.

Check out the stuff from Joe Duffy on multi-threading, the man is scary smart when it comes to this stuff.

1 Like
5

@ taylorza - The tip with volatile is a good one I thin, thank you.

About dead locks: I think that having a dead lock because of two locks is not the greatest danger.
Most of the time its that one thread enters the lock and then waits for something inside (like an Event), which is set by an other thread from inside the same lock object.
This happens mostly when I apply the locks later, because there where some instability problems I wanted to solve by this. It also happens often when using Dispatcher.Invoke() :wall:

So some good rules for locks are also:
[ul]Keep the lock block as short as possible. By this I mean code length and execution time.[/ul]
[ul]Make no method calls inside the lock, or only to pure methods when ever possible. By this you don’t oversee any potential dead lock risks[/ul]
[ul]When ever possible don’t wait for something triggered by somewhere else in your code inside the lock[/ul]

1 Like