-3

I am reading a Guid object twice, once within an if-statement, and once withing the block of the if-statement.

In C I would copy the variable locally, to make sure that the read operation inside the if-statement does not read a different value (if another thread changes this value inbetween).

I am skeptical that the following approach is a) thread safe and b) will give me the same value:

public class MyClass {

private Guid MyProp {get;set;} = Guid.Empty; //May be changed at will

public OnRunLoop() //Gets called periodicaly on a thread
{

    var ALocalCopyOfTheProp = MyProp; //Copy locally
    if(ALocalCopyOfTheProp == AValueFromAnotherPlace) //Read 1
    {
        ...

        var AnotherReadOfTheVariable = ALocalCopyOfTheProp; //Read 2

    }

    ...
}

C#.NET natively has no copy capabilities from my googling, so what is best-practice in a case such as this?

Edit: - Note that in this case MyProp is out of my hands. I can not use a lock since the property is coming from elsewhere. (apologies for not making that clearer) - Guid is a struct and not a reference type

mjwills
  • 23,389
  • 6
  • 40
  • 63
sommmen
  • 6,570
  • 2
  • 30
  • 51
  • 3
    I believe Guid is a struct, rather than a reference type – St. Pat Feb 11 '19 at 22:04
  • Personally I'd use the LOCK statement – Trey Feb 11 '19 at 22:04
  • You can make the method thread safe by placing a lock around the code in question, this will prevent other threads from entering the block until processing is finished. – Ryan Wilson Feb 11 '19 at 22:04
  • 3
    `Guid` is a value type, and you can't rely on reads to be atomic, so `var ALocalCopyOfTheProp = MyProp` wouldn't be safe either. – Lee Feb 11 '19 at 22:04
  • 1
    I don't understand. How can you not have control of `MyClass.MyProp` if you are writing `MyClass`. Use the same lock on the `MyProp` setter (the same lock you use when you consume that property), and everything should work. If the value is coming from elsewhere, they are going to hand it to your MyClass instance via the MyProp property. Just wrap the property setter with the `lock` statement. – Flydog57 Feb 11 '19 at 22:23

2 Answers2

7

In C I would copy the variable locally, to make sure that the read operation inside the if-statement does not read a different value (if another thread changes this value in-between).

I note that this technique just exchanges one problem -- inconsistent reads -- for another problem -- stale reads.

This technique is common in C# in multithreaded event handlers; you frequently see something like:

var myEvent = this.MyEvent;
if (myEvent != null) myEvent(whatever);

Though that does solve the problem of a race condition where this.MyEvent changes between the if and the invoke, it does not solve the deeper problem: you can do a check on one thread, set the event handler to null on another thread, and now the assumption that the event handler will never be called again is false, because a stale value is going to be invoked on the other thread!

You can't simply copy stuff locally and hope for the best. You have to prove that any possible reordering of your reads and writes on any number of threads produces a sensible program.

The Guid object is a reference type

It absolutely is not.

C#.NET natively has no copy capabilities from my googling, so what is best-practice in a case such as this?

Take out a lock.

I can not use a lock

Then you are required to only call your property on a single thread.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • If one can't use a lock then pretty much all of the reads are always stale, and one has to just learn to live with this somehow. Locking is way easier for one's sanity, and in non-contested scenarios doesn't hurts the performance. – Joker_vD Feb 12 '19 at 10:30
1

Guid is struct (value type) and its size is longer than atomically read values which are 32 or 64 bits depending on OS (What operations are atomic in C#?).

As result you can't read (or write) Guid value in thread-safe manner if the value can be changed from other threads. Code accessing Guid (both read and write) must be protected by some synchronization mechanism (i.e. lock).

Alternatively if you can guarantee that reads will never happen before all possible writes are to that field are complete (essentially making field immutable) you can read it without locks. You always need to protect writes to Guid value if there is more then one possible writer thread for the same field.

Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179