Search code examples
c++thread-safetylockingdeadlock

Do I need to lock around a const container with non-const elements?


I've developed a multi-threaded application (developed in C++) that has various collections in which the containers (vectors, maps, etc) are const upon initialization, but the elements in the containers are not const.

I'm wondering if I need to hold a lock the entire time while retrieving an element from the container and reading/writing to it, or if just locking when reading/writing to it is enough... I'm just trying to resolve/avoid some deadlock issues, which would be solved if I didn't have to hold a lock while iterating the collection.

Here is a simple example, my question in this example is around the locking procedures for the GetObject & AdjustValue functions. If this was a multi-threaded application, I would ideally only like to lock when calling AdjustValue and ONLY when adjusting the data of a specific element in the collection. I think this would be okay, since the collection is const (objects are not added, removed, or deleted once the collection is populated).

CustomClass.h

class CustomClass
{
public:
   CustomClass() : 
      m_value(0)
   {

   }

   int m_value;
};

CustomCollection

#include "CustomClass.h"
#include <vector>

class CustomCollection
{
public:
   CustomCollection() : 
      m_collection(CreateCustomCollection())
   {

   }

   CustomClass* GetObject(
      const size_t index)
   {
      // Should I lock here, or since the collection is const can I avoid locking?
      if(index >= 0 && index < m_collection.size())
      {
         return m_collection[index];
      }

      return nullptr;
   }

   void AdjustValue()
   {
      // Do I need to lock before calling GetObject?
      // Since the collection is const, can I lock only when adjusting the values of the object itself?
      CustomClass* custom_object = GetObject(0);
      if(custom_object)
      {
         // Ideally I only need to lock here since the collection is const, just not the objects.
         custom_object->m_value++;
      }
   }
private:
   static const std::vector<CustomClass*> CreateCustomCollection()
   {
      std::vector<CustomClass*> collection;
      collection.push_back(new CustomClass());

      return collection;
   }

   const std::vector<CustomClass*> m_collection;
};

Solution

  • m_collection is effectively a constant object, itself, that never gets modified after it is constructed. As such, no locking is required to access this container.

             // Ideally I only need to lock here since the collection is const, just not the objects.
             custom_object->m_value++;
    

    Correct, if m_value is accessed from multiple execution threads then all access to m_value must be properly synchronized.