clarification on using std::unique_lock

Question

So, I have a std::map object which is getting accessed by multiple threads in same time and I decided to use unique_lock to make the map operation safe.

In one of thread's implementation, the map object is getting used by some functions (these functions generally add/remove items from map object), So I wanted to know if defining the unique_lock at the top of parent function would be guarantee the safety ? or i need to add it to every single of those functions ?

void Thread1() { std::unique_lock<std::mutex> ul(mutex); func1(); // map object is getting changed here func2(); // map object is getting changed here }

As long as a mutex is locked it cannot be locked by any other thread which tries to lock the same mutex. Hence, this can be a "sync. penalty" which slows down your concurrent execution. The lock should enclose as tight as possible every access to the shared data. You may expose a minimal reproducible example to get a more specific answer. — Scheff's Cat
– Scheff's Cat, Commented Apr 2, 2020 at 13:50
Generally you want to create and destroy the unique_lock as close to the critical section as possible. That means put it in each of func1 and func2 only around the parts where your map is accessed. Make sure to put it around read access as well (or use a read/write lock instead this would be an underlying shared_mutex and a combination of shared_lock and unique_lock to grab access). — JohnFilleau
– JohnFilleau, Commented Apr 2, 2020 at 13:50
Thank you @Scheff , JohnFilleau for replies. But lets code optimization aside, does this logic guarantee the safety ? — atari83
– atari83, Commented Apr 2, 2020 at 14:01
Hmm. Safety against race condition? Yes. What about dead-lock? One thread tries to write but never unlocks when a buffer is filled. Another thread tries to read but is waiting on the lock. That's safe to fail (from users view). And, btw. I wouldn't struggle with all the pitfalls of multi-threading if I wouldn't intend to earn performance/speed-up by concurrency... ;-) — Scheff's Cat
– Scheff's Cat, Commented Apr 2, 2020 at 14:05
Alternatively, you may provide each thread a separate map (or buffer or whatever) to fill, and merge the results after joining all threads. So, you wouldn't need any sync. (except the one which you already have (and cannot prevent) at start and join of each thread) i.e. you wouldn't need a mutex. That could be the most safe and even fastest solution for your problem. — Scheff's Cat
– Scheff's Cat, Commented Apr 2, 2020 at 14:10

Solomon Slow · Accepted Answer · 2020-04-02 17:24:15Z

The whole point of using the mutex is to prevent threads B, C, D, etc. from seeing shared data in an inconsistent state because of changes made by thread A.

You proposed this:

void Thread1() { std::unique_lock<std::mutex> ul(mutex); func1(); // map object is getting changed here func2(); // map object is getting changed here }

Does func1() leave the map (and maybe other shared variables) in a state that other threads should not see? and does func2() make it OK again for other threads to see the shared data? If so, then your example probably is the best you can hope for. But, if each of those two function calls leaves the shared data in a "safe" state, then you might consider having each one of them separately lock and unlock the mutex.

You're usually better off if threads can be designed so that they don't often need to access shared data, and when they do need to access it, they get in and out (i.e., lock and unlock) as quickly as possible.

See also, readers writer locking (in C++, std::shared_lock) in case that happens to help with your particular problem.

West · Accepted Answer · 2020-04-02 13:56:17Z

What I see you have here can't really work. If every thread tries to grab the mutex at the start of operation either you have deadlock or sequential threads where only one thread can run or you have multiple different mutexes. The latter is not correct but the former kind of gives up some of the benefits of multithreading.

If you used more explicit scoping for the functions that have to be locked that it is fine to use locks without adding them directly to functions in use.

void Thread1() { funcA(); {// Explicit scoping std::unique_lock<std::mutex> ul(mutex); func1(); // map object is getting changed here func2(); // map object is getting changed here } funcB(); }

We want to use locks for the smallest period possible. So you have to make a judgment call on whether the functions that change the map object are small enough to justify locking for the entire time.

Collectives™ on Stack Overflow

clarification on using std::unique_lock

2 Answers 2

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Related