digital_fate
digital_fate

Reputation: 597

Is the following thread-safe?

I have the following code:

public class Info
{
    public int Data { get; set; }
}

public class Updater
{
    private static readonly object lockObject = new object();
    private Info myInfo= new Info();

    public Info MyInfo
    {
        get
        {
            lock (lockObject)
            {
                return myInfo;
            }
        }
    }

    public void UpdateInfo()
    {
        lock (lockObject)
        {
            myInfo.Data = ReadFromExternalDevice();
        }
    }        
}

I have one instance of Updater which is accessed from two separate threads.

Updater updater = new Updater();

Thread #1 periodically calls UpdateInfo().

updater.UpdateInfo();

Thread #2 periodically reads from the Data property of the Info property.

int latestData = updater.MyInfo.Data;

Is the above code in anywhere close to being thread-safe?

Upvotes: 1

Views: 63

Answers (2)

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186668

Imagine that

  • 1st thread obtains MyInfo (and leaves the lock after return myInfo;)
  • 1st thread starts reading MyInfo properties
  • 2nd thread starts writing into MyInfo
  • 1st thread keeps on reading MyInfo (partially changed!) properties

in this scenario the code is not thread safe. Even if MyInfo has just one int field it's not thread safe:

MyInfo info = Updater.Info;

Console.Write(info.Data);

// Here 2nd thread changes the info
Console.Write(info.Data);

Typical implementation uses ReaderWriterLockSlim

https://msdn.microsoft.com/en-us/library/system.threading.readerwriterlockslim(v=vs.110).aspx

  • when reading 1st thread EnterReadLock() reads up (clones) MyInfo into a local variable (or performs all the required procedures with MyInfo), then ExitReadLock()
  • when writing 2nd thread EnterWriteLock() writes, then ExitWriteLock()

Upvotes: 2

René Vogt
René Vogt

Reputation: 43876

If you only care about reading the Data property then this implementation is... let's say... thread-safe enough.

Read and write operations on 32bit values are atomic in C#, so there will be no strange intermediate value in updater.MyInfo.Data at any point. If you have more complex properties that are updated, then this no longer applies. Thread 2 may have only partially updated these properties while Thread 1 is still reading on the same MyInfo.

But if you try to do something like

if (updater.MyInfo.Data == 5)
    updater.MyInfo.Data = 7;

in different threads, this is no longer guaranteed to behave as expected. As another thread may have changed the value of Data between the check and the assignment.

You may want to have a look at Interlocked.Exchange for a thread-safe way to update a variable.

Upvotes: 1

Related Questions