Lock in using ConcurrentDictionary

דני שטרית 2,226 Reputation points
2024-01-28T08:23:05.3633333+00:00

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;
                }
            }
        }

     

      }
}



.NET
.NET
Microsoft Technologies based on the .NET software framework.
3,176 questions
{count} vote

1 answer

Sort by: Most helpful
  1. Jiale Xue - MSFT 17,521 Reputation points Microsoft Vendor
    2024-01-29T03:12:53.8266667+00:00

    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.