Obtaining lock on a UI thread

Question

Is obtaining a lock on the UI thread a bad practice? For example I have a cache object which is being modified by both background threads and the UI thread. Any modifications to the cache is wrapped inside a lock. The pseudo code is as follows:

 public class Cache
    {
        private readonly Dictionary<string, IEnumerable<string>> _cache;
        private readonly object _onCache = new object();
        public Cache()
        {
            _cache = new Dictionary<string, IEnumerable<string>>();
        }

        //Called by worker threads
        public void RefreshInBackground(IEnumerable<string> items, string key)
        {
            lock (_onCache)
            {
                _cache[key] = items;
            }
        }

        //Called by UI thread.Does this lead to any issues since UI thread is waiting on a lock?
        public void RefreshInForeground(IEnumerable<string> items, string key)
        {
            lock (_onCache)
            {
                _cache[key] = items;
            }
        }
    }

When the RefreshInForegound is called by the UI thread it has to wait on the lock (maybe), and the question is, is this a bad practice and if yes why?

Thanks, -Mike

score 4 · Answer 1 · edited Jun 20 '20 at 09:12

If it works fine like it is, I wouldn't change it.

If you have a performance concern then you can tweak it. Some points to improve:

Major

Any potentially long-running operation shall be executed in a non-UI thread. Which means you need to remove RefreshInForeground from the class and use Task, ThreadPool, BackgroundWorker etc to run RefreshInBackground from UI.

Minor

Switch to Monitor and try-finally, add a timeout. This is still not the best option as UI thread becomes blocked for at most the duration of a timeout. Still better than waiting forever.
Use optimized lockers, such as ReaderWriterLockSlim
Use ConcurrentDictionary - lockless or non-blocking class

score 1 · Accepted Answer · answered Apr 30 '13 at 10:35

When the RefreshInForegound is called by the UI thread it has to wait on the lock (maybe), and the question is, is this a bad practice and if yes why?

If the operation can take a long time, this can be a problem, since it blocks the UI.

If all operations using the same lock object are short (such as the simple dictionary setters in your example), I would not worry about it.

score 0 · Answer 3 · edited May 23 '17 at 11:57

For dictionary, there is ConcurrentDictionary class since .Net 4.0. See

Dictionary as thread-safe variable

For other type of thread-safe collection, you may see http://msdn.microsoft.com/en-us/library/dd287108.aspx

For locking question, it is not a bad practice. The lock keyword is most convenient way to manage concurrency in trivial usage, but not the most optimal. For example, ReaderWriterLock might be better if you are doing single-write multiple-read synchronisation.

Obtaining lock on a UI thread

3 Answers3

Major

Minor

Linked