How to apply InterLocked.Exchange for Enum Types in C#?

Question

public enum MyEnum{Value1, Value2}  
class MyClass 
{ 
    private MyEnum _field;   
    public MyEnum Field  // added for convenience
    {
        get { return _field; }  
        set { Interlocked.Exchange(ref _field, value); // ERROR CS0452  }
    }  
}

could be solved with:

 public enum MyEnum{Value1, Value2}  
 public class MyClass2  
 {  
   private int _field;  //change to int
   public MyEnum Field  // added for convenience
   { 
    get { return (MyEnum)_field; }
    set { System.Threading.Interlocked.Exchange(ref _field, (int)value); }
   }  
 }

Is there any better way for this problem?

There is an generic overload of Interlocked.Exchange, wouldn't that work? — harold, Aug 24 '11 at 14:43
@harold: no, that overload has a constraint limiting T to be of reference type. — Corey Kosak, Aug 24 '11 at 14:45
Since you're ignoring the return value of the `Exchange`, what value is it adding here? — Damien_The_Unbeliever, Aug 24 '11 at 14:50
yes right, didn't think of that. Anyway the result of the exchange is not used, so it's not really doing an exchange but just an assignment. Making the enum field `volatile` should be enough, right? — harold, Aug 24 '11 at 14:51

score 7 · Accepted Answer · answered Aug 24 '11 at 14:59

7

Is there any better way for this problem?

If you need to use Interlocked.Exchange then this is the best way, in fact I think it is the only way to Exchange an enum.

The reason you get the compiler error is that the compiler thinks you want to use Exchange<T>, but T needs to be a reference type for this to work, since you are not using a reference type it fails. So, the best work around is to cast to an int as you have done, and thus force the compiler to use the non-generic Exchange(int, int).

answered Aug 24 '11 at 14:59

Steve

8,469
1
26
37

Yes. I am in complete agreement with you about the usage of Interlocked, the reason for compiler error and approach to solving by casting. I just felt the code was looking ugly. – vinayvasyani Aug 24 '11 at 15:12
Beauty is in the eye of the beholder, I don't think it looks that bad:) At least the "ugly" code is hidden from client who just see the MyEnum property. I am curious as to what you are actually doing here - did you read the other answers? Ignoring the return value seems "strange" to say the least. – Steve Aug 24 '11 at 15:14
Well I dont need the return value. Interlocked as compared to lock approach suggested by Tejs was just the optimization. I would most likely take lock(syncobject) approach to solve my current problem.Thanks for your replies though. – vinayvasyani Aug 24 '11 at 15:22
Accepting your answer for my Acceptance percentage is very low and many people down vote my questions for that reason. Thanks for your answer. – vinayvasyani Aug 25 '11 at 20:31

score 5 · Answer 2 · answered Aug 24 '11 at 14:49

5

You appear to not need the "exchange" feature of Interlocked.Exchange, as you are ignoring its return value. Therefore I think the solution that might make you happiest is to mark _field as volatile:

private volatile MyEnum _field;

answered Aug 24 '11 at 14:49

Corey Kosak

2,615
17
13

2

AFAIK"volatile" does not replace Interlocked.Exchange! It just makes sure that the variable is not cached, but used directly. Incrementing a variable requires actually three operations: 1.read 2.copy 3.write Interlocked.Exchange performs all three parts as a single atomic operation – vinayvasyani Aug 24 '11 at 15:40
`volatile` in C# causes full memory fences, in contrast to C++ where it is pretty much useless. And you're only doing one operation here: write. It's true that if you did a read/modify/write, `volatile` wouldn't cut it. – harold Aug 24 '11 at 15:51
7

@harold: Volatile in C# is documented as causing **half fences** on reads and writes, not **full fences**. In practice an implementation may of course choose to provide more than a half fence, but if you rely on that then you are relying on an undocumented implementation detail that is subject to change. That said, you are of course correct to note that "volatile" and "atomic test and set" are two completely different things. – Eric Lippert Aug 24 '11 at 16:04
@Eric: Ok thanks, didn't know they were just half fences. I went to look in the disassembly what the JIT actually emitted and found .. nothing. I'm probably doing something wrong - but I couldn't make it emit any kind of memory fence at all.. – harold Aug 24 '11 at 18:43
4

@harold: Are you disassembling on an architecture with a strong memory model, like x64 or x86? On those architectures all accesses are *already* treated as volatile, so no fences need to be generated. The only effect "volatile" has on those architectures is to prevent compiler optimizations. If you want to see fences get generated then disassemble on a weak memory model chip, like Itanium. – Eric Lippert Aug 24 '11 at 18:47
@Eric: yes, but this is all hugely confusing - to me anyway. If I understand this correctly, `volatile` in C# behaves very differently from Java, and doesn't guarantee sequential consistency but instead something I don't quite get. Even on x86, an mfence would be needed after each volatile store to get sequential consistency. – harold Aug 24 '11 at 20:33
2

@harold: Volatile does not guarantee sequential consistency in C#. It guarantees acquire and release semantics. If you want to know what precisely C# guarantees, I encourage you to read the relevant sections of the C# specification. – Eric Lippert Aug 24 '11 at 22:14
@harold: It is confusing to most people (me included). But to say that it doesn't guarantee sequential consistency might be leaving something out. The spec says a volatile read must occur before all other reads & writes that occur further down in the instruction sequence. Similar, but opposite, semantics exist for volatile writes. It does not preclude volatile reads and writes from being moved per se, but it does restrict that movement quite a bit. So it does provide at least *some* sequential consistency. – Brian Gideon Aug 25 '11 at 19:26

score 3 · Answer 3 · answered Aug 25 '11 at 19:16

The Interlocked methods are fine. You could use a plain old lock, but that seems like overkill here. However, you are going to need to use a guarded read of some kind in the getter otherwise you may run into memory barrier issues. Since you are already using an Interlocked method in the setter it makes sense to do the same in the getter.

public MyEnum Field  // added for convenience
{ 
  get { return (MyEnum)Interlocked.CompareExchange(ref _field, 0, 0); }
  set { Interlocked.Exchange(ref _field, (int)value); }
}

You could also get away with marking the field as volatile if you like.

score 0 · Answer 4 · answered Sep 04 '13 at 09:12

Is there any better way for this problem?

I use a class instead of Enum:

public class DataCollectionManagerState
{
    public static readonly DataCollectionManagerState Off = new DataCollectionManagerState() { };
    public static readonly DataCollectionManagerState Starting = new DataCollectionManagerState() { };
    public static readonly DataCollectionManagerState On = new DataCollectionManagerState() { };

    private DataCollectionManagerState() { }

    public override string ToString()
    {
        if (this == Off) return "Off";
        if (this == Starting) return "Starting";
        if (this == On) return "On";

        throw new Exception();
    }
}

public class DataCollectionManager
{
    private static DataCollectionManagerState _state = DataCollectionManagerState.Off;

    public static void StartDataCollectionManager()
    {
        var originalValue = Interlocked.CompareExchange(ref _state, DataCollectionManagerState.Starting, DataCollectionManagerState.Off);
        if (originalValue != DataCollectionManagerState.Off)
        {
            throw new InvalidOperationException(string.Format("StartDataCollectionManager can be called when it's state is Off only. Current state is \"{0}\".", originalValue.ToString()));
        }

        // Start Data Collection Manager ...

        originalValue = Interlocked.CompareExchange(ref _state, DataCollectionManagerState.On, DataCollectionManagerState.Starting);
        if (originalValue != DataCollectionManagerState.Starting)
        {
            // Your code is really messy
            throw new Exception(string.Format("Unexpected error occurred. Current state is \"{0}\".", originalValue.ToString()));
        }
    }
}

score -2 · Answer 5 · answered Aug 24 '11 at 14:38

-2

Why not simply synchonrize the threading?

protected static object _lockObj = new object();

set
{
    lock(_lockObj)
    {
         _field = value;
    }
}

answered Aug 24 '11 at 14:38

Tejs

40,736
10
68
86

3

MSDN says, on modern CPU's Interlocked is optimized and is often literally a single CPU instruction. – vinayvasyani Aug 24 '11 at 14:41
this only works if every access to the field is locked, which could cause a lot of work. – Stefan Steinegger Aug 25 '11 at 10:42

How to apply InterLocked.Exchange for Enum Types in C#?

5 Answers5

Linked