c++ - Correct atomic memory order, when 2 threads update same atomic variable and both use the value in condition (C++11) -


i have 1 atomic integer variable has minimum , maximum value. 2 threads update variable, 1 increments , other decrements it. if incrementing increase value on maximum value, thread blocks , waits condition variable. same thing happens decrementing when value hits minimum. when value decremented , old value maximum value, decrementing thread should notify incrementing thread , same thing should happen other way around when incrementing.

decrementing function body:

if (atomic_var.load(std::memory_order_acquire) == minimum) {     std::unique_lock<std::mutex> lk(mutex);     if (atomic_var.load(std::memory_order_acquire) == minimum) {         //we have hit minimum have wait other thread increase variable         condition_var.wait(lk, [&]() {             return atomic_var.load(std::memory_order_relaxed) > minimum;         });     }      //do stuff      std::atomic_fetch_sub_explicit(&atomic_var, 1u, std::memory_order_release);     lk.unlock();     condition_var.notify_all();     return; }  //do stuff  if (std::atomic_fetch_sub_explicit(&atomic_var, 1u, std::memory_order_release) == maximum) {     //we have hit maximum other thread might waiting     std::atomic_thread_fence(std::memory_order_acquire);     condition_var.notify_all(); } //adding condition_var.notify_all() here fixes problem i'm afraid //that causes bit great performance penalty when avoided

which memory orderings should use these checks? current implementation seems cause dead locks...

edit: changing memory orders std::memory_order_seq_cst doesn't seem remove problem.

the atomic memory access isn't problem. don't have test case prove it, think... 

if (atomic_var.load(std::memory_order_acquire) == minimum) {     // **your deadlock here**     std::unique_lock<std::mutex> lk(mutex);     if (atomic_var.load(std::memory_order_acquire) == minimum) {

consider:

  1. the outer test minimum successful , enters.
  2. before sub can acquire mutex, add thread scheduled.
  3. add loads maximum number jobs queue (or whatever) , acquires mutex , waits.
  4. sub cannot acquire mutex , blocks until can released.
  5. no subtraction = no notification = no unlock

one simple solution ditch outer test minimum, grab mutex, test minimum , wait if necessary. 

void consume() {     std::unique_lock<std::mutex> lock(mutex, std::defer_lock);     (;;)     {         lock.lock();         if (atomic_var.load(std::memory_order_acquire) <= minimum)         {             condition_var.wait(lock, [&]()             {                 return atomic_var.load(std::memory_order_relaxed) > minimum;             });         }         lock.unlock();         // stuff         std::atomic_fetch_sub_explicit(&atomic_var,                                        1,                                        std::memory_order_release);         condition_var.notify_all();     } }

i moved , deferred unique_lock rid of unnecessary create-destroy cycle. moved unlock few lines because there's no point locking out add thread during time consuming operation.


-