Safe use of a reference in a map in multi-threaded context

Question

The context

I have a class (let's say Foo) managing some centralized ressources as an history in a static map, with an accessor to read them and a function to add new data (there is no way to remove a key) :

class Foo
{
  private:
    static std::map<std::string,MyDataStructure> data;
  public:
    static const MyDataStructure& getData(const std::string& key)
    {
      assert(Foo::data.count(key) > 0); // Must exist
      return Foo::data[key];
    }
    static void addData(const std::string& key, const MyDataStructure& d)
    {
      assert(Foo::data.count(key) == 0); // Can not already exist
      Foo::data[key] = d;
    }
};

In order to avoid concurrency problems, I added a mutex I manage like that :

class Foo
{
  private:
    static std::map<std::string,MyDataStructure> data;
    static boost::mutex mutex_data;
  public:
    static const MyDataStructure& getData(const std::string& key)
    {
      boost::mutex::scoped_lock lock(Foo::mutex_data);
      assert(Foo::data.count(key) > 0); // Must exist
      return Foo::data[key];
    }
    static void addData(const std::string& key, const MyDataStructure& d)
    {
      boost::mutex::scoped_lock lock(Foo::mutex_data);
      assert(Foo::data.count(key) == 0); // Can not already exist
      Foo::data[key] = d;
    }
};

My questions

My first question is about the reference to Foo::data returned by getData : this reference is used out of the scope of the mutex, then is it possible to have a problem with that ? Is it possible to loose the reference due to another access adding data ? In brief : is the reference always the same once in the map ?
If yes, is the assert required in addData ? Can the reference change if I change data linked to an existing key in the map ?
Is the lock required in getData ? I think maybe not if std::map is already multi-thread safe.

Are you using asserts here because your program logic disallows a call to `getData` before adding the data or are you using it to check that the data exists in the map? — Mohamad Elghawi, May 18 '16 at 11:01
@MohamadElghawi : it's not really disallowed, but clearly not the good way to use the object. In my program `Foo::data` is an history about creation of objects having a uniqid. There is no sense to try to access to the history of an object that doesn't exist. I use `assert` because it's not a big matter if an empty history is created and returned in Release mode. — Caduchon, May 18 '16 at 11:06
If it's not disallowed then I would suggest using something other than an assert. Asserts are generally not compiled into the release build and so your check will not cease to exist in production. — Mohamad Elghawi, May 18 '16 at 11:09
It's exaclty what I said : it's not required in release mode. It's an error for the developper using the object, not for the user of the application. (see design-by-contract programming) — Caduchon, May 18 '16 at 11:11
Actually, I also have a `static bool hasKey(const std::string& key)` function designed for the user to check if the key exist before calling it. The user have the responsability to only call getData with existing keys, and he has the tool to do it. But exposing this function in my question was not required. — Caduchon, May 18 '16 at 11:14

score 2 · Accepted Answer · edited May 23 '17 at 12:19

Is is possible to loose the reference due to another access adding data ? In brief : is the reference always the same once in the map?

See Lifetime of references in STD collections

"for std::map, the references are valid as long as you don't clear the map, or erase the specific referenced element; inserting or erasing other elements is fine."

Can the reference change if I change data linked to an existing key in the map?

Pursuant to the rule above, no...modifying a reference already in the map is just tweaking the bits at that same address. (As long as you don't implement modification via removal of the key and then re-adding that key again.)

Is the lock required in getData ? I think maybe not if std::map is already multi-thread safe.

See C++11 STL containers and thread safety

So the lock is required, because the "internal wiring" of the map structure may be in flux during addData()'s insertion...that wiring might be tripped on during the lookup traversal for getData() without a guard.

However, you can be reading or writing one of the already-present MyDataStructure references during an add or removal, if you have the reference in your hand. It's just that process of navigating from map-to-reference that needs to be sure no one is writing during the navigation.

Safe use of a reference in a map in multi-threaded context

1 Answers1