Lock in using ConcurrentDictionary

Question

Hi, I have phenomenon of deadlock using this code. I'm using net 8 with worker service that host scheduler of net that work in multi -thread enviorement. I have dead lock it hppened from time to time in RoutesStatusSingelton class. a.Can I removed the lock in GetRouteStatus /UpdateStatus sine I used thread safe collection- ConcurrentDictionary ? b.Can i replace remove and add to one operation update- and no need for lock in GetRouteStatus /UpdateStatus. if yes please give full code. Calling: 1.

  // Check if route is running if yes exit to next tick
  if (RoutesStatusSingelton.GetInstance().GetRouteStatus(routeData.Name))
  {
      _logger.Debug($"GUID:{emailGuid}, route name: {routeData.Name} is running, not schedule again.");
      continue;
  }
  else
  {
      _logger.Debug($"GUID:{emailGuid}, route name: {routeData.Name} is start running.");

  }

       RoutesStatusSingelton.GetInstance().UpdateStatus(routeData.Name, false);
                                 

using AutoamtionService.Shared.Helpers;
using System.Collections.Concurrent;

namespace AutomationClient.Shared.Helpers
{
    /// <summary>
    /// The class mange the status of each scheduled route.
    /// The status can be running(true) or not running(false)
    /// </summary>
    public class RoutesStatusSingelton
    {

        /// <summary>
        /// The class instance as singelton
        /// </summary>
        private static RoutesStatusSingelton _routesStatus;
        /// <summary>
        /// The data structure the key is route name and value is running or not
        /// </summary>
        private static ConcurrentDictionary<string, bool> _dic = new ConcurrentDictionary<string, bool>();
        /// <summary>
        /// The lock object
        /// </summary>
        private static readonly object _syncObject = new object();
     

        private RoutesStatusSingelton()
        {
           
        }

        /// <summary>
        /// Get one insatnce of RoutesStatusSingelton using double-check locking
        /// </summary>
        /// <returns>Returns one insatnce of RoutesStatusSingelton using double-check locking</returns>
        public static RoutesStatusSingelton GetInstance()
        {
           if (_routesStatus == null)
            {
                lock (_syncObject)
                {
                    if (_routesStatus == null)
                    {
                        _routesStatus = new RoutesStatusSingelton();
                    }
                }
            }
            return _routesStatus;
        }


        /// <summary>
        /// Get the status of runnings routes:runing or not
        /// </summary>
        /// <param name="routeName">The route name</param>
        /// <returns>Returns the status of route name true for running  or false for start running</returns>
        public bool GetRouteStatus(string routeName)
        {

            lock (_syncObject)
            {
                if (_dic.TryGetValue(routeName, out bool isRunning))
                {
                    if (!isRunning)
                    {
                        if (_dic.TryRemove(routeName, out bool oldStatus))
                        {
                            _dic.TryAdd(routeName, true);
                            MongoDbHelper.InsertRouteActivationDocument(routeName, true.ToString());
                            return false; // strat first time to run
                        }
                        else
                        {
                            return false; // case of failure at if
                        }

                    }
                    else
                    {
                        return true;//is running
                    }

                }
                else //  case of failure at if
                {
                    _dic.TryAdd(routeName, true);
                    MongoDbHelper.InsertRouteActivationDocument(routeName, true.ToString());
                    return false;
                }
            }
        }


        /// <summary>
        /// Update status of running thread
        /// </summary>
        /// <param name="routeName">The route name<param>
        /// <param name="newStatus">The new satus value</param>
        /// <returns>Return true if status succdeed otherwise return false</returns>
        public bool UpdateStatus(string routeName, bool newStatus)
        {

            lock (_syncObject)
            {
                if (_dic.TryRemove(routeName, out bool oldStatus))
                {
                    MongoDbHelper.InsertRouteActivationDocument(routeName, newStatus.ToString());
                    return _dic.TryAdd(routeName, newStatus);
                }
                else
                {
                    return false;
                }
            }
        }

     

      }
}

Answer 1

Hi @דני שטרית , Welcome to Microsoft Q&A,

It seems like you're using a ConcurrentDictionary to store the route statuses, which is already designed to be thread-safe for individual operations. However, in your code, you still use a lock (_syncObject) when accessing and modifying the dictionary. If you want to take full advantage of the thread-safety provided by ConcurrentDictionary, you can remove the locks from the GetRouteStatus and UpdateStatus methods. Try to use the code bellow:

using AutomationService.Shared.Helpers;
using System.Collections.Concurrent;

namespace AutomationClient.Shared.Helpers
{
    public class RoutesStatusSingleton
    {
        private static RoutesStatusSingleton _routesStatus;
        private static ConcurrentDictionary<string, bool> _dic = new ConcurrentDictionary<string, bool>();

        private RoutesStatusSingleton()
        {
        }

        public static RoutesStatusSingleton GetInstance()
        {
            if (_routesStatus == null)
            {
                lock (_syncObject)
                {
                    if (_routesStatus == null)
                    {
                        _routesStatus = new RoutesStatusSingleton();
                    }
                }
            }
            return _routesStatus;
        }

        public bool GetRouteStatus(string routeName)
        {
            if (_dic.TryGetValue(routeName, out bool isRunning))
            {
                if (!isRunning)
                {
                    if (_dic.TryRemove(routeName, out bool oldStatus))
                    {
                        _dic.TryAdd(routeName, true);
                        MongoDbHelper.InsertRouteActivationDocument(routeName, true.ToString());
                        return false; // start first time to run
                    }
                    else
                    {
                        return false; // case of failure at if
                    }
                }
                else
                {
                    return true; // is running
                }
            }
            else // case of failure at if
            {
                _dic.TryAdd(routeName, true);
                MongoDbHelper.InsertRouteActivationDocument(routeName, true.ToString());
                return false;
            }
        }

        public bool UpdateStatus(string routeName, bool newStatus)
        {
            if (_dic.TryRemove(routeName, out bool oldStatus))
            {
                MongoDbHelper.InsertRouteActivationDocument(routeName, newStatus.ToString());
                return _dic.TryAdd(routeName, newStatus);
            }
            else
            {
                return false;
            }
        }
    }
}

Best Regards,

Jiale

---

If the answer is the right solution, please click "Accept Answer" and kindly upvote it. If you have extra questions about this answer, please click "Comment". 

Note: Please follow the steps in our documentation to enable e-mail notifications if you want to receive the related email notification for this thread.