0

Is it possible to get a thread safe property by using Interlocked in the property accessors?

Example:

public class A()
{
    private static long count;

    public static long Count
    {
        get
        {
            return Interlocked.Read(ref count);
        }
        set
        {
           Interlocked.Exchange(ref count, value); 
        }
    }
}
Ben Steeves
  • 130
  • 1
  • 10
  • 3
    "Thread safe" is a property of an entire program, not just one snippet of it, and is also not a particularly meaningful term until you specifically and explicitly define what you mean by "thread safe", meaning what conditions will/will not be met throughout your program. Having said that, I would say that generally speaking it's code smell to see a property like that. It's a sign to me that someone thinks something is going to work properly when in fact it will not, although one would need to look at every single usage of the property to know for sure. – Servy May 28 '14 at 19:50
  • The only thing one can say about the 'thread safety' of your example code is that, the value of Count is one that was actually assigned to it by the property set (what cannot be said if you replace the Interlocked with simple assignation and return). However, as Servy said, it doesn't mean a lot if you do not define exactly what 'thread safety' properties are you talking about. – MaMazav May 28 '14 at 19:54
  • 1
    @MaMazav You can't even say that, really. It's possible for `Interlocked.Read` to return a value, have it be stored in a temporary, have the setter be called, changing the value, and then have the getter return, so you can't even say that the getter doesn't return a stale value. The added memory barrier helps it not be *as* stale, and prevents the re-ordering of gets and sets *from the same thread*, but when multiple threads are involved, you can still do some pretty funky things as a result of storing the value in a temporary (which is done implicitly) before its returned. – Servy May 28 '14 at 19:59
  • 1
    Atomicity and thread-safety are distinct concepts, the atomicity guarantee you got from this code is a very weak one. All you have is a promise that you'll never read a partially updated value when your program runs in 32-bit mode. – Hans Passant May 28 '14 at 20:18
  • @Servy My only claim was that the code he wrote promise that a returned value must be a one that was assigned to it in the past (and not partially assigned, as HansPassant said). I don't understand how storing the value in a temporary breaks it. – MaMazav May 30 '14 at 05:01
  • @MaMazav It means that the guarantees provided by the language are very likely not to be sufficient for the OP, and that they won't ensure that his code works. Those guarantees are simply very weak, and the OP likely doesn't realize how weak they are. – Servy Jun 02 '14 at 14:09

2 Answers2

1

When the above example is run, the execution behavior of the get and set accessors is linearizable. Without the use of Interlocked, the execution behavior of the get and set accessors is somewhere between weak consistency and sequential consistency (i.e. only guaranteed to exhibit weak consistency).

When run as a 64-bit process, the same could be accomplished by marking the field volatile and using a simple return statement and assignment operator. When run as a 32-bit process, however, the operations on a volatile 64-bit field are not guaranteed to be atomic, so the use of Interlocked is required to ensure atomicity.

Sam Harwell
  • 97,721
  • 20
  • 209
  • 280
  • 3
    So... Is that a Yes or a No ? – H H May 28 '14 at 20:11
  • @HenkHolterman The question does not make sense in this context, and as such cannot have a simple yes or no answer. From the perspective of outside code interacting with the property in question, my answer is precise. – Sam Harwell May 28 '14 at 20:25
  • @280Z28 When a question is not answerable due to a lack of information you should vote to close it, not give an incomplete and rather unhelpful answer. – Servy May 28 '14 at 20:51
  • @280Z28 At this point you've given some reasonable statement as to what guarantees are made by this code. The problem is that we have no idea whatsoever if those guarantees are actually sufficient for the OP's purposes, having no idea what they are. Combined with the fact that most people don't actually understand the concepts such as "weak consistency" and "sequential consistency" that you've mentioned, the probability that a reader can determine if their code is valid given this answer seems...low in my mind. – Servy May 29 '14 at 13:53
1

It is not thread safe. Try this code:

long i
{
    get { return Interlocked.Read(ref _i); }
    set { Interlocked.Exchange(ref _i, value); }
}


long _i;

void Main()
{

    Parallel.ForEach(Enumerable.Range(0, 1000_000), 
        //new ParallelOptions { MaxDegreeOfParallelism = 1},
        x=>
    {
        i++;
    });

    i.Dump();
}

When you run this code the answer is not 1000_000, but a bit lower proving it is not thread safe. Not sure why this happens

Alfons
  • 511
  • 4
  • 17
user168345
  • 167
  • 2
  • 8
  • 1
    ++ postfix will capture the value of i and have it increased by one. More than one thread can safely read the value of i before it gets increased (after it is read). And later those different threads will "safely" write the same value on top of i. – Ünsal Ersöz Jan 09 '20 at 14:06