What is wrong with std::lock_guard

  • A+
Category:Languages

I have simple code: first thread pushes std::strings to the std::list, and second thread pops std::strings from this std::list. All std::lists operations are protected with std::mutex m. This code permanently prints error to console: "Error: lst.begin() == lst.end()".

If I replace std::lock_guard with construction m.lock() and m.unlock() the code begins work correctly. What is wrong with std::lock_guard?

#include <iostream> #include <thread> #include <mutex> #include <list> #include <string>  std::mutex m; std::list<std::string> lst;  void f2() {     for (int i = 0; i < 5000; ++i)     {         std::lock_guard<std::mutex> { m };         lst.push_back(std::to_string(i));     }      m.lock();     lst.push_back("-1"); // last list's element     m.unlock(); }  void f1() {     std::string str;      while (true)     {         m.lock();         if (!lst.empty())         {             if (lst.begin() == lst.end())             {                 std::cerr << "Error: lst.begin() == lst.end()" << std::endl;             }             str = lst.front();             lst.pop_front();             m.unlock();             if (str == "-1")             {                 break;             }         }         else         {             m.unlock();             std::this_thread::yield();         }     } }  // tested in MSVS2017 int main() {     std::thread tf2{ f2 };     f1();     tf2.join(); } 

 


You did not obey CppCoreGuidelines CP.44: Remember to name your lock_guards and unique_locks :).

In

for (int i = 0; i < 5000; ++i) {     std::lock_guard<std::mutex> { m };     lst.push_back(std::to_string(i)); } 

you are only creating a temporary std::lock_guard object which is created and destroyed immediately. You need to name the object like in

{     std::lock_guard<std::mutex> lg{ m };     lst.push_back(std::to_string(i)); } 

so that the lock guard lives until the end of the block.

And as you already recognized (CppCoreGuidelines):

Use RAII lock guards (lock_guard, unique_lock, shared_lock), never call mutex.lock and mutex.unlock directly (RAII)

Comment

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen: