Monitor Wait for caching,... it this a good practice?

Question

i just wrote a code, and then i found out that there are some issue with the monitor.wait, forcing me to do the operation within the locks, i wanted to now if this is a good way to keep a thread waiting,....

i'm not sure if the thread.join would do the job, as there are lots of threads running within my application, and each do specific job, that they may terminate within the time...

here is my code:

public static class TaskManager
{
    private static readonly object UpdateLock = new object();
    private static readonly object WaitLock = new object();

    private static readonly LiaisonDb _db = new LiaisonDb();
    private static List<liaQueue> _liaQueueList = new List<liaQueue>();
    private static DateTime _lastUpdate = new DateTime();

    public static liaQueue GetTask(string sessionType)
    {
        liaQueue task;
        lock (UpdateLock)
        {
            if (_lastUpdate < DateTime.Now.AddSeconds(-5))
            {
                Thread t = new Thread(UpdateCache) {IsBackground = true};
                t.Start();
                lock (WaitLock)
                {
                    Monitor.Wait(WaitLock);
                }

                _lastUpdate = DateTime.Now;
            }
            task = _liaQueueList
                .FirstOrDefault(w => w.Stat == 0
                                     && w.Type != null
                                     || string.Equals(w.Type, sessionType));
        }
        return task;
    }

    private static void UpdateCache()
    {
        try
        {
            _liaQueueList = _db.liaQueue.Where(w => w.Stat == 0).ToList();
        }
        finally
        {
            lock (WaitLock)
            {
                Monitor.Pulse(WaitLock);
            }
        }
    }
}

As you see i put two lock, and one of them is only for monitor.wait, keep the thread waiting for the answer...

i think i also have to returns null while the cache is getting refreshed?...

Answer 1

From MSDN

If two threads are using Pulse and Wait to interact, this could result in a deadlock.

So, no. Your implementation would not be best practice.

It seems to me that GetTask is supposed to update the cache on a background thread, then block the calling thread until the cache was updated, and then return the first task according to a select criteria.

Since the calling thread will block (wait) for the cache to be updated, I don't quite understand the point of using a background thread in the first place. If the purpose is to prevent multiple calling threads to update the cache in parallel, use just the lock(UpdateLock) statement.

If you do want to run the cache on a background thread anyway (and wait for it), consider using the Task library instead. But I don't really se the point of it.

lock (UpdateLock)
{
  if (_lastUpdate < DateTime.Now.AddSeconds(-5)) {
    Task.Run(() => {
      _liaQueueList = _db.liaQueue.Where(w => w.Stat == 0).ToList();
    }).Wait();

    _lastUpdate = DateTime.Now;
  }
}

return _liaQueueList.FirstOrDefault(w => w.Stat == 0 && w.Type != null || string.Equals(w.Type, sessionType));