Is writing std::atomic thread safe in the area of read lock

Question

I've a std::map<std::string, std::tuple<int, int, std::atomic<int>>> mp. The map can be read and write parallel so I use a read-write lock to keep it thread safe.

I want to know if I can write std::get<2>(mp["someKey"]) in the read lock area.

void read() {
    // read lock area
    std::shared_lock<std::shared_mutex> read_lock{mtx};
    // "someKey" exists already
    std::get<2>(mp["someKey"]).store(2);
}

I've tried to do some tests, it seems that it works but I don't know if it's incorrect and gives me a coredump someday.

Answer 1

Firstly, note that mp["someKey"] modifies the map and is not thread-safe. If no key exists yet, operator[] will create a new value-initialized key/value pair. See also Why is std::map::operator[] considered bad practice?

Instead, you should use mp.at("someKey") or mp.find("someKey"):

// OK, exception thrown if key doesn't exist yet
std::get<2>(mp.at("someKey")).store(2);

// OK, nothing happens if key doesn't exist yet
if (auto it = mp.find("someKey"); it != mp.end()) {
    std::get<2>(*it).store(2);
}

.store(2); is thread-safe because writing to a std::atomic from multiple threads simultaneously is safe. The std::shared_lock isn't necessary to ensure this. However, the std::shared_lock ensures that the surrounding std::pair cannot be destroyed by another thread so it must be kept.

Note on const-correctness

Const-correctness would have prevented the bug with mp["someKey"]. If you only hold a read-lock, it's good practice to work with a const& to prevent accidental modifications:

std::as_const(mp)["someKey"]); // error

const auto& cmp = mp;
cmp["someKey"]; // error

Note on std::tuple

std::tuple outside of variadic templates is always questionable. It's almost always better to have meaningful names:

struct entry {
    int a, b; // obviously, use better names than a, b, c
    std::atomic<int> c;
};

// ...

if (auto it = mp.find("someKey"); it != mp.end()) {
    it->c.store(2);
}