Issue with ManualResetEvent not releasing all wait

Question

I'm trying to implement a class which uses a simple cache for holding data retrieved from an internal service. I'm using a ManualResetEvent to block multiple threads which may try to refresh the cached data at the same time with the first thread to succeed signalling the others to proceed once the data has been retrieved by calling Set() and then Reset(). When testing I've noticed that sometimes all of the threads are released and sometimes 1 or more are not and are left to time out, almost as if I am calling Reset before all of the threads were released. Can someone explain what I am doing wrong?

I've included a cut down version of the code below.

    private bool _updating;
    private const int WaitTimeout = 20000;
    private DateTime _lastRefresh;
    private object _cacheData;
    private readonly ManualResetEvent _signaller = new ManualResetEvent(false);

private void RefreshCachedData()
        {
            Console.WriteLine("ThreadId {0}: Refreshing Cache", Thread.CurrentThread.ManagedThreadId);
        if (_updating)
        {
            Console.WriteLine("ThreadId {0}: Cache Refresh in progress, waiting on signal.", Thread.CurrentThread.ManagedThreadId);

            // another thread is currently updating the cache so wait for a signal to continue
            if (!_signaller.WaitOne(WaitTimeout))
                Console.WriteLine("ThreadId {0}: Thread timed out ({1}s) waiting for a signal that the cache had been refreshed",
                    Thread.CurrentThread.ManagedThreadId,WaitTimeout);

            Console.WriteLine("ThreadId {0}: Signal recieved to use refreshed cache.", Thread.CurrentThread.ManagedThreadId);
        }
        else
        {
            try
            {
                _updating = true;

                var result = _requestHandler.GetNewData();

                if (!result.Success)
                {
                        Console.WriteLine("Failed to retrieve new data.");
                }
                else
                {
                    // switch the cache with the new data
                    _cacheData = result.Data;

                    Console.WriteLine(
                        "ThreadId {0}: Cache refresh success.",
                        Thread.CurrentThread.ManagedThreadId);
                    Thread.Sleep(8000);
                }
            }
            catch (Exception ex)
            {
                Console.WriteLine("Error occured: {0}", ex);
            }
            finally
            {
                // Set the refresh date time regardless of whether we succeded or not
                _lastRefresh = DateTime.Now;
                _updating = false;

                // signal any other threads to to carry on and use the refreshed cache
                Console.WriteLine("ThreadId {0}: Signalling other threads that cache is refreshed.", Thread.CurrentThread.ManagedThreadId);
                _signaller.Set();
                _signaller.Reset();
            }
        }
    }

Answer 1

Looks like your threads are not getting released from the ResetEvent before it is reset.

You could solve the problem by creating the event open and having the fist thread to enter your method reset it.

Alternatively you can avoid the vagaries of ManualResetEvent's behavior by doing something like this:

private object _latch = new object();
private bool _updating;

private void UpdateIfRequired() 
{
    lock (_latch) 
    {
        if (_updating) 
        {
            //wait here and short circuit out when the work is done
            while (_updating)
                Monitor.Wait(_latch);

            return;
        }

        _updating = true;
    }

    //do lots of expensive work here

    lock (_latch)
    {
        _updating = false;
        Monitor.PulseAll(_latch); //let the other threads go
    }
}

Check out this page for a great explanation as to how this works http://www.albahari.com/threading/part4.aspx#_Signaling_with_Wait_and_Pulse