ReaderWriterLock not working in ServiceBehavior constructor

Question

I have a WCF service where InstanceContextMode is Single and ConcurrencyMode is Multiple. The aim is to create a cache of values on instantiation, without holding up other service calls not reliant upon the creation of the cache.

This way only methods that attempt to obtain a read lock on _classificationsCacheLock will need to wait until the value of classificationsCache is populated (classificationsCacheLock.IsWriterLockHeld = false).

However the issue is, despite acquiring a write Lock in the task thread, the call WCF continues to service in response to a call into the service method GetFOIRequestClassificationsList() results in _classificationsCacheLock.IsWriterLockHeld being false, when it should be true.

Is this odd behavior with WCF instancing or is this me fundamentally missing a trick.

I tried both acquiring the write lock within the thread context of the constructor (safe option) and within the context of the spawned task thread (which could introduce a race between WCF then invoking the call to the GetFOIRequestClassificationsList() function faster than call to classificationsCacheLock.AcquireWriterLock(Timeout.Infinite);) but both result in classificationsCacheLock.IsWriterLockHeld being false despite having prevented any race condition via the use of thread.sleep, staggered appropriately apart in the code blocks of each respective thread.

[ServiceBehavior(Namespace = Namespaces.MyNamespace,
    ConcurrencyMode = ConcurrencyMode.Multiple,
InstanceContextMode = InstanceContextMode.Single)]
[AspNetCompatibilityRequirements(RequirementsMode = AspNetCompatibilityRequirementsMode.Allowed)]
public class MyService : IMyService
{
    private static NLog.Logger _logger = NLog.LogManager.GetCurrentClassLogger();

    private List<string> _classificationsCache;

    private ReaderWriterLock _classificationsCacheLock;

    public MyService()
    {
        try
        {
            _classificationsCacheLock = new ReaderWriterLock();
            LoadCache();
        }
        catch (Exception ex)
        {
            _logger.Error(ex);
        }

    }

    private void LoadCache()
    {
        // _classificationsCacheLock.AcquireWriterLock(Timeout.Infinite); 

        Task.Factory.StartNew(() =>
        {
            try
            {
                _classificationsCacheLock.AcquireWriterLock(Timeout.Infinite); // can only set writer on or off on same thread, not between threads
                if (_classificationsCache == null)
                {
                    var cases = SomeServices.GetAllFOIRequests();
                    _classificationsCache = cases.SelectMany(c => c.Classifications.Classification.Select(cl => cl.Group)).Distinct().ToList();
                }

            }
            catch (Exception ex)
            {

                _logger.Error(ex);

            }
            finally
            {

                if (_classificationsCacheLock.IsWriterLockHeld)
                    _classificationsCacheLock.ReleaseWriterLock();
            }

        });//.ContinueWith((prevTask) =>
        //{
        //     if (_classificationsCacheLock.IsWriterLockHeld)
        //          _classificationsCacheLock.ReleaseWriterLock();
        //  });

    }

     public GetFOIRequestClassificationsList_Response GetFOIRequestClassificationsList()
    {
        try
        {

            GetFOIRequestClassificationsList_Response response = new GetFOIRequestClassificationsList_Response();

            _classificationsCacheLock.AcquireReaderLock(Timeout.Infinite);
            response.Classifications = _classificationsCache;
            _classificationsCacheLock.ReleaseReaderLock();

            return response;

        }
        catch (Exception ex)
        {
            _logger.Error(ex);

            if (ex is FaultException)
            {
                throw;
            }
            else
                throw new FaultException(ex.Message);
        }
    }
}

EDIT 1

Since a number of suggestions were around uncertainty within the thread pool and how a Task might handle thread affinity, I changed the method to explicitly spawn a new thread

var newThread = new Thread(new ThreadStart(() =>
        {
            try
            {
                Thread.Sleep(2000);

                Debug.WriteLine(string.Format("LoadCache - _classificationsCacheLock.GetHashCode - {0}", _classificationsCacheLock.GetHashCode()));
                Debug.WriteLine(string.Format("LoadCache - Thread.CurrentThread.ManagedThreadId-  {0} ", Thread.CurrentThread.ManagedThreadId));


                _classificationsCacheLock.AcquireWriterLock(Timeout.Infinite); // can only set writer on or off on same thread, not between threads
                if (_classificationsCache == null)
                {
                    var cases = SomeServices.GetAllFOIRequests();
                    _classificationsCache = cases.SelectMany(c => c.Classifications.Classification.Select(cl => cl.Group)).Distinct().ToList();
                }

            }
            catch (Exception ex)
            {

                _logger.Error(ex);

            }
            finally
            {

                if (_classificationsCacheLock.IsWriterLockHeld)
                    _classificationsCacheLock.ReleaseWriterLock();
            }

        }));
        newThread.IsBackground = true;
        newThread.Name = "MyNewThread" 
        newThread.Start();

The result is still the same. classificationsCacheLock.AcquireReaderLock does not wait / block as it seemingly should.

I also added some diagnostics to check if;

• The thread was in fact the same thread, you cannot expect a R/W to block on the same thread

• The instance of the _classificationsCacheLock was identical at all times

  public GetFOIRequestClassificationsList_Response GetFOIRequestClassificationsList() { try {

          GetFOIRequestClassificationsList_Response response = new GetFOIRequestClassificationsList_Response();
  
          Debug.WriteLine(string.Format("GetFOIRequestClassificationsList - _classificationsCacheLock.GetHashCode - {0}", _classificationsCacheLock.GetHashCode()));
          Debug.WriteLine(string.Format("GetFOIRequestClassificationsList - Thread.CurrentThread.ManagedThreadId - {0} ", Thread.CurrentThread.ManagedThreadId));
  
          Thread.Sleep(1000);
  
           _classificationsCacheLock.AcquireReaderLock(Timeout.Infinite);
          //_classificationsCacheMRE.WaitOne();
  
          response.Classifications = _classificationsCache;
  
          _classificationsCacheLock.ReleaseReaderLock();
  
          return response;
  
      }
      catch (Exception ex)
      {
          _logger.Error(ex);
  
          if (ex is FaultException)
          {
              throw;
          }
          else
              throw new FaultException(ex.Message);
      }
  }
  

The result was ..

GetFOIRequestClassificationsList - _classificationsCacheLock.GetHashCode - 16265870
GetFOIRequestClassificationsList - Thread.CurrentThread.ManagedThreadId - 9 
LoadCache - _classificationsCacheLock.GetHashCode - 16265870
LoadCache - Thread.CurrentThread.ManagedThreadId-  10  

.. in that order so now we have a race condition as expected since the write lock was obtained in the newly created thread. The actual WCF Service call is being made before the constructor's spawned thread has been scheduled to actually run. So I move

  _classificationsCacheLock.AcquireWriterLock(Timeout.Infinite);

to the constructor since this is guaranteed to be executed before any class fields are accessed.

Still the AcquireWriterLock does not block, despite the evidence that the constructor was initialized on a different WCF thread to the WCF thread that executes the service method.

 private void LoadCache()
    {

        _classificationsCacheLock.AcquireWriterLock(Timeout.Infinite);

        Debug.WriteLine(string.Format("LoadCache constructor thread - _classificationsCacheLock.GetHashCode - {0}", _classificationsCacheLock.GetHashCode()));
        Debug.WriteLine(string.Format("LoadCache constructor thread - Thread.CurrentThread.ManagedThreadId-  {0} ", Thread.CurrentThread.ManagedThreadId));

        var newThread = new Thread(new ThreadStart(() =>
        {
            try
            {
                Thread.Sleep(5000);

                Debug.WriteLine(string.Format("LoadCache new thread - _classificationsCacheLock.GetHashCode - {0}", _classificationsCacheLock.GetHashCode()));
                Debug.WriteLine(string.Format("LoadCache new thread - Thread.CurrentThread.ManagedThreadId-  {0} ", Thread.CurrentThread.ManagedThreadId));


              //  _classificationsCacheLock.AcquireWriterLock(Timeout.Infinite); // can only set writer on or off on same thread, not between threads
                if (_classificationsCache == null)
                {
                    var cases = SomeServices.GetAllFOIRequests();
                    _classificationsCache = cases.SelectMany(c => c.Classifications.Classification.Select(cl => cl.Group)).Distinct().ToList();
                }

            }
            catch (Exception ex)
            {

                _logger.Error(ex);

            }
            finally
            {

                if (_classificationsCacheLock.IsWriterLockHeld)
                    _classificationsCacheLock.ReleaseWriterLock();
            }

        }));
        newThread.IsBackground = true;
        newThread.Name = "CheckQueues" + DateTime.Now.Ticks.ToString();
        newThread.Start();
}

Again AcquireWriterLock does not block and allows a null reference classificationsCache to be assigned.

The result was ..

LoadCache constructor thread - _classificationsCacheLock.GetHashCode - 22863715
LoadCache constructor thread - Thread.CurrentThread.ManagedThreadId-  9 
GetFOIRequestClassificationsList - _classificationsCacheLock.GetHashCode - 22863715
GetFOIRequestClassificationsList - Thread.CurrentThread.ManagedThreadId - 8 
LoadCache new thread - _classificationsCacheLock.GetHashCode - 22863715
LoadCache new thread - Thread.CurrentThread.ManagedThreadId-  10 

EDIT 2

Created a copy of the solution without source control.

Uploaded here if you want to have hands on with the issue.

Changed to using manual reset event in code for demo purposes and commented out problem code.

• Place your breakpoints
• run Example.Web in Debug
• in the browser navigate to 'http://localhost:11164/GetFOIRequestClassificationsList.htm' and click the button.

MRE works, ReaderWriterLock does not work as expected.

.net 4.0 - C#

Answer 1

In the CaseWork() method you are creating new ReaderWriterLock each time the method is being called. So the lock being aquired simply goes out to the Garbage COllector, and the new one arises. Therefore no lock is actually being correctly aquired.

Why don't you use static lock for this, with creating it in a static constructor?

Correct me if I wrong, but I can't if you are updating your cache. If this is true, I suggest you to simply use the Lazy<T> class. It's thread-safe, and holds all readers before the value is set. It internally uses the TPL, and is simply to use:

private Lazy<List<string>> _classificationsCache = new Lazy<List<string>>
(() => 
{
    var cases = SomeServices.GetAllFOIRequests();
    return cases.SelectMany(c => c.Classifications.Classification.Select(cl => cl.Group)).Distinct().ToList();
});

You can get the value like this:

response.Classifications = _classificationsCache.Value;

Update from MSDN:

If the current thread already has the writer lock, no reader lock is acquired. Instead, the lock count on the writer lock is incremented. This prevents a thread from blocking on its own writer lock. The result is exactly the same as calling AcquireWriterLock, and an additional call to ReleaseWriterLock is required when releasing the writer lock.

I think that this thing is happen:

Your reader lock aquiring is being fired inside the same thread (Task.StartNew method uses the TaskScheduler.Current property) the writer lock is working, so it doesn't block as it has the same priveleges as the Task do, and got the empty list. So in your circumstances you have to choose another one synchronization primitive.