Making Custom Object Thread Safe

Question

I posted an earlier question about returning collections, and the topic of thread safety came up. I was given this link to do some more reading, and I found this particular line:

In general, avoid locking on a public type, or instances beyond your code's control.

First, correct me if I'm wrong, but doesn't the example that Microsoft give lock on a public type, the balance variable?

Secondly, how would I go about locking my own getter/setter property. Suppose I have the following class:

private int ID;

public Person(int id)
{
    this.Identification= id;

}

public int Identification
{
    get { return this.ID; }

    private set
    {
        if (value == 0)
        {
            throw new ArgumentNullException("Must Include ID#");
        }
        this.ID = value;
    }
}

The getter is public correct? Only the setter is declared private. So, how would I lock, or make my getter/setter properties thread safe?

Answer 1

When you need to lock on a variable, you need to lock around every place where the variable is used. A lock is not for a variable - it's for a region of code where a variable is used.

It doesn't matter if you 'only read' in one place - if you need locking for a variable, you need it everywhere where that variable is used.

An alternative to lock is the Interlocked class - this uses processor-level primitives for locking that's a bit faster. Interlocked, however cannot protect multiple statements (and having 2 Interlocked stataments is not the same as having those 2 statements inside a single lock).

When you lock, you must lock on an instance of a reference type (which, in most cases (but not always), should also be a static instance). This is to ensure that all locks are actually taken out on the same instance, not a copy of it. Obviously, if you're using a copy in different places, you're not locking the same thing so your code won't be correctly serialized.

For example:

private static readonly object m_oLock = new object ();

...

lock ( m_oLock )
{
    ...
}

Whether it's safe to use a non-static lock requires detailed analysis of the code - in some situations, it leads to more parallelism because the same region of code is locked less but the analysis of it could be very tricky - if you're unsure, just use a static lock object. The cost of taking an open lock is minimal but incorrect analysis may lead to errors that take ages to debug.

Edit:

Here's an example showing how to lock property access:

private int ID; // do NOT lock on value type instances
private static readonly object Lock = new object ();

public Person(int id)
{
    this.Identification = id;
}

public int Identification
{
    get
    { 
        lock ( Lock )
        {
            return this.ID;
        }
    }

    private set
    {
        if (value == 0)
            throw new ArgumentNullException("Must Include ID#");

        lock ( Lock )
        {
            this.ID = value;
        }
    }
}

Since your property only does a trivial get/set operation, you can try using Interlocked.CompareExchange instead of a full lock - it will make things slightly faster. Keep in mind, though, that an interlocked operation is not the same as a lock.

Edit 2:

Just one more thing: a trivial get / set on an int doesn't need a lock - both reading and writing a 32-bit value (in and of itself) is already atomic. So this example is too simple - as long as you're not trying to use ID in multiple operations that should be completed in an atomic fashion, the lock is not needed. However, if your real code is actually more complicated with ID being checked and set, you may need locking and you'll need to lock around all the operations that make up the atomic operation. This means that you may have to pull the lock out of the getter / setter - 2 locks on a get/set pair of a variable is not the same as a single lock around them.

Answer 2

you should define a variable in Person class like this

private readonly object _lock_ = new Object();

if you want to make synchronization over all instances of Person you should make it static.

then when you want to lock you can do it like this

lock(_lock_) //whose there? it's me, I kill you! oops sorry that was knock knock
{
    //do what you want
}

I suggest you to read this article: 1

Answer 3

The answer to your first question about the Microsoft article: No. The article doesn't lock on the balance variable. It locks on the private thisLock variable. So the example is good.

Secondly, based on the code you have posted, you don't need to add any locking to make your class thread safe, because your data is immutable. Once you create an instance of Person and set the value for the Identification property from within the constructor, your class design doesn't allow for that property to change again. That's immutability, and that in itself provides thread safety. So you don't need to bother with adding locks and such. Again, assuming your code sample is accurate.

EDIT: This link may be useful to you.