Is that synchro-construction correct ?

Question

I have a multi-threaded application. Threads use ABC.Connector. I want that only one thread at a time have access to a Connector property.

class ABC
{
    /// <summary>
    /// Synchronization object.
    /// </summary>
    static object _syncObject = new object();

    static DataAccess _connector;
    /// <summary>
    /// Global object for data access.
    /// </summary>
    public static DataAccess Connector
    {
        get
        {
            lock (_syncObject)
            {
                return _connector.CreateCopy(); // get a copy of original _connector
            }
        }
        set
        {
            lock (_syncObject)
            {
                _connector = value;
            }
        }
    }
}

Is it correct ?

Answer 1

Well, that will certainly make getting and setting the Connector property thread-safe (although I'd make _syncObject read-only). However, it doesn't make DataAccess thread-safe... the mutex will only apply while threads are getting and setting the property.

In other words, if two threads both do:

ABC.DataAccess.DoSomeLongRunningOperation();

then DoSomeLongRunningOperation() will still be run concurrently by the two threads. If that operation isn't thread-safe, it's still going to be a problem.

If you only want one thread at a time to use the DataAccess then you could write:

public static void UseConnector(Action<DataAccess> action)
{
    lock (_syncObject)
    {
        action(_connector);
    }
}

Then if two threads both do:

ABC.UseConnector(access => access.DoLongRunningOperation());

then DoLongRunningOperation() will only run in one thread at a time. You've still got the issue that misbehaving clients could write:

DataAccess naughty = null;
ABC.UseConnector(access => naughty = access);
// Haha! I've circumvented your thread safety!
naughty.DoLongRunningOperation();

... but hopefully that isn't an issue for you.

Answer 2

Yes, this is generally correct. But take into account, that after Connector get returns the _connector reference, access to it is unsynchronized.