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