Reordering operations around volatile

Question

Currently, I am looking at implementing a set of copies for a record and want to confirm its safety by stream. I am sure that the only way this may not be the case is with the compiler allowing the reordering of statements in certain methods. For example, the Remove method looks like this:

 public bool Remove(T item) { var newHashSet = new HashSet<T>(hashSet); var removed = newHashSet.Remove(item); hashSet = newHashSet; return removed; } 

Where hashSet is defined as

 private volatile HashSet<T> hashSet; 

So my question is, does hashSet volatile mean that Remove in the new set happens before it is written to the member variable? If not, then other threads may see the set before deletion.

I have not seen any problems with this in production, but I just want to confirm that it will be safe.

UPDATE

To be more specific, there is another way to get IEnumerator :

 public IEnumerator<T> GetEnumerator() { return hashSet.GetEnumerator(); } 

So a more specific question: is there any guarantee that the returned IEnumerator will never throw a ConcurrentModificationException from the delete?

UPDATE 2

Sorry, all answers relate to thread safety from multiple authors. Good points come up, but that's not what I'm trying to figure out here. I would like to know if the compiler is allowed to reorder operations in Remove something like this:

  var newHashSet = new HashSet<T>(hashSet); hashSet = newHashSet; // swapped var removed = newHashSet.Remove(item); // swapped return removed; 

If possible, this would mean that the thread could call GetEnumerator after the hashSet was assigned, but before the item was deleted, which could cause the collection to change during the enumeration.

Joe Duffy has which says:

Volatile on loads means ACQUIRE, no more, no less. (There are additional limitations to compiler optimization, of course, as well as not allowing you to go beyond the limits of loops, but it allows you to focus on aspects of MM at the moment.) The standard definition for ACQUIRE is the following memory operations cannot be moved before the ACQUIRE statement; for example, given {ld.acq X, ld Y}, ld Y cannot happen before ld.acq X. However, previous memory operations can certainly move after it; for example, given {ld X, ld.acq Y}, ld.acq Y can really happen before ld X. Currently, only one Microsoft.NET processor code works, for which it is actually IA64, but this is a noticeable area where the CLR MM is weaker than most machines. Further, all .NET stores are RELEASE (regardless of volatility, i.e. no-op volatility in terms of jitted code). The standard definition of RELEASE is that previous memory operations cannot be moved after a RELEASE operation; for example, given {st X, st.rel Y}, st.rel Y cannot happen before st X. However, subsequent memory operations can actually move in front of it; for example, given {st.rel X, ld Y}, ld Y can move up to st.rel X.

As I read this, calling newHashSet.Remove requires ld newHashSet , and writing to hashSet requires st.rel newHashSet . From the RELEASE definition above, no loads can be moved after the RELEASE repository, so instructions cannot be reordered! Can someone confirm, please confirm that my interpretation is correct?

Answer 1

Edition:

Thanks for clarifying the existence of an external lock for Remove calls (and other mutations in the collection).

Due to the RELEASE semantics, you cannot save the new value until hashSet until the value of the removed variable is assigned (since st removed cannot be moved after st.rel hashSet ).

Consequently, the behavior of the “snapshot” GetEnumerator will work as intended, at least with respect to Remove and other mutators implemented in a similar way.

Answer 2

Consider using Interlocked.Exchange - this will guarantee order, or Blocked .CompareExchange , which as a side benifit will allow you to detect (and possibly restore) the simultaneous recording to the collection. Obviously, it adds some additional level of synchronization, so it is different from your current code, but easier to reason about.

 public bool Remove(T item) { var old = hashSet; var newHashSet = new HashSet<T>(old); var removed = newHashSet.Remove(item); var current = Interlocked.CompareExchange(ref hashSet, newHashSet, old); if (current != old) { // failed... throw or retry... } return removed; } 

And I think that in this case you still need volatile hashSet .

Answer 3

I can’t speak for C #, but in C volatile basically points and indicates only that the contents of a variable can change at any time. It does not contain restrictions on reordering the compiler or processor. All you get is that the compiler / processor will always read the value from memory, and not trust the cached version.

I believe, however, that in the recent MSVC (and possibly C #), reading volatility acts as a memory barrier for loads and write operations as a memory barrier for stores, for example. the processor will stop until all loads have been completed, and no loads can avoid this by being redirected below a mutable read (although later independent loads can still move up above the memory barrier); and the processor will stop until the stores are completed, and no stores can avoid it by being reordered below the volatile recording (although subsequent independent recordings may still move up above the memory barrier).

When only one thread writes a given variable (but many read it), only memory barriers are required for proper operation. When multiple threads can be written to a given variable, atomic operations should be used because the design of the CPU is such that there is a basic race condition in the record, so that the record may be lost.