Thread safety across shared instances (C#)

Question

I use a factory pattern to create a custom object which is loaded from a cache if possible. There are no static members or functions on the custom object.

Assuming 2 threads call the factory and are both returned references to the same object from the cache. (i.e. No new operator, in ref to answer below, object returned from a collection)

If I want to modify a private instance member on inside the class:

a) Shoulb I lock it first? b) Will the change be reflected in both threads?

I assume yes for both questions, but at the same time it feels like the threads have different instances of the class.

Have I must something fundamental here? Why do I feel like I have?

===============

Following the first few answers I have confirmed what I thought, thanks.

I guess what I really want to know is, if the objects are pretty much only read-only, i.e. after being created they only have one instance member that can change, do I need to do any locks when reading properties that are not affected by that one changeable instance member?

Again I assume no, but I have to come to value the second-opinion of the collective StackOverflow brains trust :)

Answer 1

If the new operator or the constructor is getting called twice on the object, then you have yourself two instances of the same class.

Answer 2

You are assuming that the threads have different instances of the object, which is incorrect. The threads are pointing to the same object; therefore, you have to synchronize access to any members that might be accessed by multiple threads, whether it be through property accesses, method calls, etc, etc.

Answer 3

It should be the same object, and the answer to your questions is YES, YES.

How do you check that there are 2 instances?

Answer 4

Yes, you should lock before setting the private member. This will be reflected in both threads (since they're the same object).

However, you may want to take some time to consider how to handle the locks and threading.

If your factory is creating the object and adding it to the collection, it will need a lock of some form around the routine to create the object. Otherwise, it's possible that the two threads could ask for the object at the same time, before it was created, and create 2 instances.

Also, I wouldn't recommend setting the private member directly - it would be a good idea to have a single object inside of the returned class instance used just for locking, and use some form of accessor (method or property) that locks on that synchronization object and then sets the field.

As for not "trusting" that its two instances - you can always just do something to check in the debugger. Add a check for reference equality, or even temporarily add a GUID to your class that's setup at construction - it's easy to verify that they're the same that way.

Answer 5

Do you need both threads to see the results of changing the private member? Or is it perfectly OK with you if both threads get the object, but when one thread changes a private member, the other thread doesn't see that change?

Here's why I ask: In your factory, you could take the object from cache and then clone or copy it before returning it. As a result, each thread would have its own copy of the class, and that copy would have the state that was in the cached object at the time they asked for it. No 2 threads would ever share the exact same instance, because you're making a copy for each thread.

If that fits your requirements, then you can avoid having to lock the object (you will need synchronization on the factory method itself though ... which you also need in your current case).

Be careful with the idea of a "clone", however. If you have a graph of objects and you only do a shallow clone, you'll wind up with references that are still shared across threads, forcing you to need synchronization again. This idea of one-copy-per-thread makes best sense if the object is a very simple object without a lot of references to other objects.

Answer 6

Reading and writing of most primitive types, string and object references is atomic (everything except non-volatile longs and doubles). This means locks are not necessary if you are simply reading or writing a field or other variable in isolation. So, if you ever see code like this (and I guaranteee you will), you can happily strip out the unnecessary and costly locking:

public SomeClass SomeProperty
{
    get
    {
        lock (someLock)
        {
            return someField;
        }
    }

    set
    {
        lock (someLock)
        {
            someField = value;
        }
    }
}

If, however, you want to change the value, increasing it by one for example, or if you want to read multiple, related variables then you need to lock if you want sensible results.

It is also worth noting that the Hashtable and Dictionary support mulitple concurrent readers without the need for any kind of locking.