title: Should volatile and readonly be mutually exclusive?
body: <p>Suppose that I were designing a thread-safe class that wraps an internal collection:</p>
  
  <pre><code>public class ThreadSafeQueue&lt;T&gt;
  {
      private readonly Queue&lt;T&gt; _queue = new Queue&lt;T&gt;();
  
      public void Enqueue(T item)
      {
          lock (_queue)
          {
              _queue.Enqueue(item);
          }
      }
  
      // ...
  }
  </code></pre>
  
  <p>Based on <a href="https://stackoverflow.com/q/38881722/1149773">my other question</a>, the above implementation is buggy, as race hazards may arise when its initialization is performed concurrently with its usage:</p>
  
  <pre><code>ThreadSafeQueue&lt;int&gt; tsqueue = null;
  
  Parallel.Invoke(
      () =&gt; tsqueue = new ThreadSafeQueue&lt;int&gt;(),
      () =&gt; tsqueue?.Enqueue(5));
  </code></pre>
  
  <p>The code above is acceptably non-deterministic: the item may or may not be enqueued. However, under the current implementation, it is also broken, and can give rise to unpredictable behaviour, such as throwing <code>IndexOutOfRangeException</code>, <code>NullReferenceException</code>, enqueueing the same item multiple times, or getting stuck in an infinite loop. This occurs since the <code>Enqueue</code> call might run <em>after</em> the new instance has been assigned to the local variable <code>tsqueue</code>, but <em>before</em> the initialization of the internal <code>_queue</code> field completes (or appears to complete). </p>
  
  <p>Per <a href="http://csharpindepth.com/Articles/General/Singleton.aspx#dcl" rel="nofollow noreferrer">Jon Skeet</a>:</p>
  
  <blockquote>
    <p><strong>The Java memory model doesn't ensure that the constructor completes before the reference to the new object is assigned to instance</strong>. The Java memory model underwent a reworking for version 1.5, but double-check locking is still broken after this without a volatile variable (<strong>as in C#</strong>).</p>
  </blockquote>
  
  <p>This race hazard could be resolved by adding a memory barrier to the constructor:</p>
  
  <pre><code>    public ThreadSafeQueue()
      {
          Thread.MemoryBarrier();
      }
  </code></pre>
  
  <p>Equivalently, it could be more concisely resolved by making the field volatile:</p>
  
  <pre><code>    private volatile readonly Queue&lt;T&gt; _queue = new Queue&lt;T&gt;();
  </code></pre>
  
  <p>However, the latter is forbidden by the C# compiler:</p>
  
  <pre><code>'Program.ThreadSafeQueue&lt;T&gt;._queue': a field cannot be both volatile and readonly
  </code></pre>
  
  <p>Given that the above seems like a justifiable use-case for <code>volatile readonly</code>, is this limitation a flaw in the language design?</p>
  
  <p>I'm aware that one could simply remove the <code>readonly</code>, since it does not affect the public interface of the class. However, that is beside the point, since the same could be said for <code>readonly</code> in general. I'm also aware of the existing question “<a href="https://stackoverflow.com/q/396385/1149773">Why readonly and volatile modifiers are mutually exclusive?</a>”; however, that tackled a different issue.</p>
  
  <p><strong>Concrete scenario</strong>: This issue seems to affect code in the <code>System.Collections.Concurrent</code> namespace of the .NET Framework Class Library itself. The <a href="http://referencesource.microsoft.com/#mscorlib/system/Collections/Concurrent/ConcurrentQueue.cs,6b5dfd0fa704e80d" rel="nofollow noreferrer"><code>ConcurrentQueue&lt;T&gt;.Segment</code></a> nested class has several fields that are only ever assigned within the constructor: <code>m_array</code>, <code>m_state</code>, <code>m_index</code>, and <code>m_source</code>. Of these, only <code>m_index</code> is declared as readonly; the others cannot be – although they should – since they need to be declared as volatile to meet the requirements of thread-safety.</p>
  
  <pre><code>private class Segment
  {
      internal volatile T[] m_array;                  // should be readonly too
      internal volatile VolatileBool[] m_state;       // should be readonly too
      private volatile Segment m_next;
      internal readonly long m_index; 
      private volatile int m_low;
      private volatile int m_high; 
      private volatile ConcurrentQueue&lt;T&gt; m_source;   // should be readonly too
  
      internal Segment(long index, ConcurrentQueue&lt;T&gt; source)
      {
          m_array = new T[SEGMENT_SIZE];              // field only assigned here
          m_state = new VolatileBool[SEGMENT_SIZE];   // field only assigned here
          m_high = -1;
          m_index = index;                            // field only assigned here
          m_source = source;                          // field only assigned here
      }
  
      internal void Grow()
      {
          // m_index and m_source need to be volatile since race hazards
          // may otherwise arise if this method is called before
          // initialization completes (or appears to complete)
          Segment newSegment = new Segment(m_index + 1, m_source);
          m_next = newSegment;
          m_source.m_tail = m_next;
      }
  
      // ...
  }
  </code></pre>