CODEWITHSUNDEEP
c#.netmultithreadingthread-safety
reggaemahn
reggaemahn

Reputation: 6648

Return Property if not null in thread safe manner

My question is really an extension to this SO question about testing a property is null before returning it. I have a similar situation:

public class MyClass
    {
        private readonly string _Name { get; set; }    
        private readonly IEnumerable<T> _Values { get; set; }    
        private IEnumerable<T> _MyProp { get; private set; }    
        public IEnumerable<T> MyProp
        {
            get
            {
                if(_MyProp == null)
                {
                    this.SetProp();
                }    
                return this._MyProp;
            }    
            private set; 
        }

        public MyClass(string Name, IEnumerable<T> Values)
        {
            this._Name = Name;
            this._Values = Values;
        }

        private void SetProp()
        {
              // Business logic using Name and Values
              this._MyProp = resultOfLogic;
        }

    }

The accepted answer to the linked SO question mentions that this is not a thread safe way of doing it. Can someone advise why it isn't and if there's a way to do this in a thread safe manner?

Upvotes: 2

Views: 288

Answers (3)

Tim Williams
Tim Williams

Reputation: 883

I think there may be an error in the code you posted. The fragment

    public IEnumerable<T> MyProp
    {
        get
        {
            if(MyProp == null)
            { // ...

is infinitely recursive and will cause a stack overflow (uncapitalised!).

Did you mean the last line to use _Values as a backing field and test that for null instead of MyProp?

Upvotes: 0

GvS
GvS

Reputation: 52518

If another thread is running, this thread can call SetProp() between the test and the call to SetProp() on your thread.

I use code like this, to make it more safe:

   // Dedicated object to lock for this property only
   private object myPropSync = new object();
   private T _myPropVal;
   public IEnumerable<T> MyProp
    {
        get
        {
            // Check if property is null
            if(_myPropVal== null)
            {
                // If null -> make sure you are the only one in the next section
                lock (myPropSync) {
                    // Re-test, because another thread can 
                    // set the property while waiting for the lock
                    if (_myPropVal== null) {
                         this.SetProp();
                    }
                }
            }    
            return this._myPropVal;
        }    
        private set {
          lock (_myPropSync) {
             _myPropVal = value;
          }
        }
    }

Upvotes: 4

Dennis
Dennis

Reputation: 37760

Can someone advise why

Imagine, that there are two threads, which execute get_MyProp in parallel. Then it is possible to get this sequence:

  • T1 : _MyProp == null -> true
  • T2 : _MyProp == null -> true
  • T1 : this.SetProp(); -> _MyProp is initialized
  • T2 : this.SetProp(); -> T2 rewrites _MyProp value, that was calculated by T1

if there's a way to do this in a thread safe manner

Convert SetProp to return IEnumerable<T> instead of setting field, and use Lazy<T> (by default, initialization will be thread-safe):

private IEnumerable<T> CalcProp()
{
    // Business logic using Name and Values
    return resultOfLogic;
}

public IEnumerable<T> MyProp 
{
    get { return _MyProp.Value; }
}
private readonly Lazy<IEnumerable<T>> _MyProp;

public MyClass(string Name, IEnumerable<T> Values)
{
    this._Name = Name;
    this._Values = Values;
    this._MyProp = new Lazy<IEnumerable<T>>(CalcProp);
}

Upvotes: 2

Related Questions