Search code examples
c#.netmultithreadingclr

Do we have to use `CompareExchange` rather than `Exchange` in a hybridLock lock that uses AutoResetEvent?


The example below is from CLR via C# book that mimic a hybrid lock:

internal sealed class AnotherHybridLock : IDisposable 
{
   // The Int32 is used by the primitive user-mode constructs (Interlocked methods)
   private Int32 m_waiters = 0;

   // The AutoResetEvent is the primitive kernel-mode construct
   private AutoResetEvent m_waiterLock = new AutoResetEvent(false);

   // This field controls spinning in an effort to improve performance
   private Int32 m_spincount = 4000; // Arbitrarily chosen count
   
   // These fields indicate which thread owns the lock and how many times it owns it
   private Int32 m_owningThreadId = 0, m_recursion = 0;

   public void Enter() {
      // If calling thread already owns the lock, increment recursion count and return
      Int32 threadId = Thread.CurrentThread.ManagedThreadId;
      if (threadId == m_owningThreadId) { 
        m_recursion++; 
        return; 
      }

      // The calling thread doesn't own the lock, try to get it
      SpinWait spinwait = new SpinWait();
      for (Int32 spinCount = 0; spinCount < m_spincount; spinCount++) {
         // If the lock was free, this thread got it; set some state and return
         if (Interlocked.CompareExchange(ref m_waiters, 1, 0) == 0) // <-----------------here
            goto GotLock;

         // Black magic: give other threads a chance to run in hopes that the lock will be released
         spinwait.SpinOnce();
      }

      // Spinning is over and the lock was still not obtained, try one more time
      if (Interlocked.Increment(ref m_waiters) > 1) {
         // Still contention, this thread must wait
         m_waiterLock.WaitOne();   // Wait for the lock; performance hit
         // When this thread wakes, it owns the lock; set some state and return
      }

       GotLock:
           // When a thread gets the lock, we record its ID and indicate that the thread owns the lock once
           m_owningThreadId = threadId; m_recursion = 1;         
   }

   public void Leave() {
      // If the calling thread doesn't own the lock, there is a bug
      Int32 threadId = Thread.CurrentThread.ManagedThreadId;
      if (threadId != m_owningThreadId)
         throw new SynchronizationLockException("Lock not owned by calling thread");

      // Decrement the recursion count. If this thread still owns the lock, just return
      if (--m_recursion > 0) return;

      m_owningThreadId = 0; // No thread owns the lock now

      // If no other threads are waiting, just return
      if (Interlocked.Decrement(ref m_waiters) == 0)
         return;

      // Other threads are waiting, wake 1 of them
      m_waiterLock.Set(); // Bad performance hit here
   }

   public void Dispose() { m_waiterLock.Dispose(); }
}

I can understand how the AnotherHybridLock works but have a question on this line:

if (Interlocked.CompareExchange(ref m_waiters, 1, 0) == 0) 
  goto GotLock;

can we use Exchange as:

if (Interlocked.Exchange(ref m_waiters, 1) == 0) 
  goto GotLock;

as it looks a little bit simple, I mean if ref m_waiters is already 1, there is no harm to "overwrite" it to 1 again.

I understand that to be rigorous in the logic, Interlocked.CompareExchange(ref m_waiters, 1, 0) == 0 is correct, but I want to make sure Interlocked.Exchange(ref m_waiters, 1) == 0 also works, so that I know my understanding on the usage of Interlocked methods is correct


Solution

  • "Yes".

    From this operation:

    Interlocked.Increment(ref m_waiters)
    

    we know m_waiters could be 2, 26, or something else. We don't want to lose that value by blindly stomping it to 1. The CEX is purely to do something special in the "we're the first waiter" scenario, and that requires CEX.

    If the field was only ever 0 or 1, then we might be able to switch. Alternatively, we could perhaps just use an interlocked increment, and not use either exchange or compare-exchange.