In my application, multiple threads need to access to a map object for inserting new items or reading the existing items. (There is no 'erase' operation).
The threads uses this code for accessing map elements:
struct PayLoad& ref = myMap[index];
I just want to know do I still need to wrap this block of this code inside of mutex ? Or is it safe to not using mutex for this purpose ?
Thanks.
CodePudding user response:
Since there is at least one write operation, i.e. an insert, then you need to have thread synchronization when accessing the map. Otherwise you have a race condition.
Also, returning a reference to the value in a map is not thread-safe:
struct PayLoad& ref = myMap[index];
since multiple threads could access the value, and at least one of them could involve a write. That would also lead to a race condition. It is better to return the value by value like this:
Payload GetPayload(int index)
{
std::lock_guard<std::mutex> lock(mutex);
return myMap[index];
}
where mutex
is an accessible std::mutex
object.
Your insert/write operation also needs to lock the same mutex:
void SetPayload(int index, Payload payload)
{
std::lock_guard<std::mutex> lock(mutex);
myMap[index] = std::move(payload);
}
CodePudding user response:
TLDR: If you're using std::map
for your data there are no guarantees about memory allocation. You should expect elements to be moved to other memory locations whenever you add new elements / resize the map, thus breaking any multi-threaded code without mutexes.
This line by itself will most likely be optimized away by the compiler, so it really depends on what you're doing with the reference afterwards. Do you read from the map element or do you write anything back into memory?
Even when reading you will need to account for other threads writing to the data while your thread reads out information, so generally you will always want to use some kind of mutex / synchronization mechanism to prevent another thread writing to your memory even if your thread is only reading from it.
If you can guarantee that the memory never changes after its been written, it is is save to access without mutex, but this assumes:
- You do not use any container with dynamic memory allocation
- When you read out you only read elements that are guaranteed to be fully initialized (index is not currently being written to on another thread)
- None of the payload data points to other un-guarded dynamic memory
Without seeing more of your backing code it's impossible to tell whether those assumptions can safely be made. With everything you provided so far, I would discourage against leaving out mutexes.
Example (theoretically threadsafe)
// Shared code
Payload myMap[32]; // Use c-style array to prevent dynamic memory allocation
int index = 0;
std::mutex mutex;
// Thread A (write)
// assumption: never wraps around or writes to old indices
// assumption: myMap[0] is initalized before Thread B is started
{
std::lock_guard<std::mutex> guard(mutex);
myMap[index 1] = Payload(...);
index;
}
// Thread B
{
struct PayLoad& ref = myMap[index];
}
You see, I made quite a few assumptions with this example. Breaking any one of them could break thread safety.