c++ - Which part should I lock when dealing with both read and write? -

i'm playing c++(11) stl , got following problem. basic idea code is: have "trigger" function, "add" function , flag(false default). if flag false "add" function's going push threadid queue, otherwise it's going insert threadid set. when trigger function called, set flag "true" , move threadids queue set. initialized 100 threads , use 1 of thread run trigger function(in code it's thread no.30). ideally result should have 0 elements in queue , 99 elements in set. however, result correct, missed numbers in set, , got exc_bad_access error. help? thank you.

#include <iostream> #include <thread> #include <vector> #include <unordered_set> #include <queue> #include <mutex> #include <atomic> using namespace std;   bool flag = false; queue<int> q; unordered_set<int> s; mutex mu;  void trigger() {     mu.lock();     flag = true;     mu.unlock();     while( !q.empty() ){         s.insert(q.front());         q.pop();     } }  void add(int id) {     mu.lock();     if( !flag )          q.push(id);     else {         if ( s.find(id) == s.end() ){             s.insert(id);         }     }     mu.unlock(); }  void missing() {     cout << "missing numbers: ";     (int = 1; <= 100; i++) {         if( s.find(i) == s.end() )              cout << << " ";     }     cout << endl; }  int main() {     vector<thread> threads;      (int = 0; < 100; i++){         if ( == 29 ) threads.push_back(thread(trigger));         else threads.push_back(thread(add, i+1));     }     (int = 0; < 100; i++){         threads[i].join();     }     cout << "q size: " << q.size() << endl;     cout << "s size: " << s.size() << endl;     missing(); } 

you have 1 thread executing trigger function , many threads executing add function. furthermore, take care guard of shared state not of it. see comments/questions in below code snippets.

void trigger() {     // 'flag' protected concurrent acceess     mu.lock();     flag = true;     mu.unlock();      // why isn't 'q' or 's' protected lock?     while( !q.empty() ){         s.insert(q.front());         q.pop();     } }  void add(int id) {     // in function both 'q' , 's' protected concurrent access     mu.lock();     if( !flag )          q.push(id);     else {         if ( s.find(id) == s.end() ){             s.insert(id);         }     }     mu.unlock(); } 

possible solution

in general, should protect state being accessed concurrently. i'd recommend using lock type (e.g., lock_guard) instead of locking , unlocking mutex directly (research raii why encouraged).

#include <iostream> #include <mutex> #include <queue> #include <thread> #include <unordered_set> #include <vector>  using namespace std;  bool flag = false; queue<int> q; unordered_set<int> s; mutex mu;  void trigger() {     lock_guard<mutex> lock(mu);     flag = true;      while (!q.empty())     {         s.insert(q.front());         q.pop();     } }  void add(int id) {     lock_guard<mutex> lock(mu);      if (!flag)     {         q.push(id);     }     else     {         if (s.find(id) == s.end())         {             s.insert(id);         }     } }  void missing() {     cout << "missing numbers: ";     (int = 1; <= 100; ++i)     {         if (s.find(i) == s.end())         {             cout << << " ";         }     }     cout << endl; }  int main() {     vector<thread> threads;     (int = 0; < 100; ++i)     {         if (i == 29)         {             threads.push_back(thread(trigger));         }         else         {             threads.push_back(thread(add, + 1));         }     }      (int = 0; < 100; ++i)     {         threads[i].join();     }      cout << "q size: " << q.size() << endl;     cout << "s size: " << s.size() << endl;     missing();      return 0; }