Is this code exceptions handled correctly?

Question

In the following code, it is possible that event throws exception and it may be not handled in even handler, (rare but its still the case)

I want keep "lck2" unlocked while executing the event, because I don't want main thread block for "mtx2", reason is nothing more than optimization.

Can I guarantee that "lck2" is always released in catch block? or there could be runtime exceptions and therefore it may cause deadlocks or some unexpected behavior?

std::unique_lock<std::mutex>lck2(mtx2); // lock used for waiting for event.

while (_isRunning) 
{
    try
    {
        while (_isRunning)
        {
            // cvar2 is condition variable
            cvar2.wait(lck2, [&] {return invoke; }); // wait until invoke == true

            if (invoke) // if event must be invoked
            {
                lck2.unlock();
                OnEvent(this, someproperty); // may throw exception
                lck2.lock();

                invoke = false; // execution completed
            }
        }
    }
    catch (...) // we need to keep this thread alive at all costs!
    {            
        lck2.lock(); // is this safe?
        invoke = false;
    }
}

Answer 1

A rewrite of your code would probably be more appropriate, to make it easier for another developer to work on the code. I will show you two rewrites:

• First, (Bad)

  while (true)
  {
      try
      {
          {
               std::lock_guard<std::mutex> lckx(mtx2);
               if(!_isRunning)
                     break;    //out of the main loop
          }
  
          bool should_invoke = false;
          {
                  std::unique_lock<std::mutex> lck2(mtx2);
                  cvar2.wait(lck2, [&] {return invoke; });
                  should_invoke = invoke;
          }     
  
          if (should_invoke) // if event must be invoked
          {
              OnEvent(this, someproperty); // may throw exception
              {
                  std::lock_guard<std:mutex> lckx(mtx2);
                  invoke = false; // execution completed
              }
          }
      }
      catch (...) // we need to keep this thread alive at all costs!
      {            
          std::lock_guard<std:mutex> lckx(mtx2);
          invoke = false;
      }
  }
  

---

• Second, (Good)

  Breaking the (first) code into smaller functional units; we also note that the expression cvar2.wait(lck2, [&]{ return invoke; }) will suspend execution and only return if woken up and invoke is true, then we can infer that we only need that expression to wait. Hence we can discard the superfluous use of invoke. Hence we have:

  void do_work(){
      while(is_running()){
          try{
               wait_for_invocation();
               OnEvent(this, someproperty); // may throw exception
               set_invocation_state(false);
          catch(...){
               set_invocation_state(false);
          }
      }
  }
  

  Where the helpers are defined:

  bool is_running(){
      std::lock_guard<std::mutex> lckx(mtx2);
      return _isRunning;
  }
  
  void wait_for_invocation(){
      std::unique_lock<std::mutex> lck2(mtx2);
      cvar2.wait(lck2, [&] {return invoke; });
  }
  
  void set_invocation_state(bool state){
      std::lock_guard<std::mutex> lckx(mtx2);
      invoke = state;
  }