9

We know that std::deque::front() return a reference to a first element of deque. I would to know if this code is always safe:

//deque of lambdas
deque<function<void(void)>> funs;

// then is some other place:
// take a lock
m.lock();
auto f = move(funs.front()); // move the first lambda in f
funs.pop_front(); // remove the element from deque //now the value is hold by f
m_.unlock(); // unlock the resorce
f(); //execute f

I've tried this code using gcc-4.9 and works but I don't know if we can consider this code safe!

Praetorian
  • 106,671
  • 19
  • 240
  • 328
Gian Lorenzo Meocci
  • 1,136
  • 2
  • 14
  • 23
  • 1
    It is almost a valid code. Almost - because you are not checking for emptiness. The moving of the stored element is a safe operation. – bobah Jun 15 '14 at 20:39
  • 1
    Typo report: You use `lock()` on `m` and `unlock()` on `m_`. – Notinlist Feb 13 '17 at 13:27
  • The biggest safety issue might be "using namespace std" ;) See also https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice – Chris Jul 02 '22 at 11:07

1 Answers1

10

The std::function move constructor is not guaranteed to not throw exceptions, so you have an exception safety issue. Since you are not using an RAII lock for m, it will remain locked if auto f = move(funs.front()); throws. You can correct the problem with std::unique_lock:

std::unique_lock<decltype(m)> lock{m};
if (!funs.empty()) {
  auto f = move(funs.front()); // move the first lambda in f
  funs.pop_front(); // remove the element from deque //now the value is hold by f
  lock.unlock(); // unlock the resorce
  f(); //execute f
}

or std::lock_guard:

function<void()> f;
{
  std::lock_guard<decltype(m)> lock{m};
  if (!funs.empty()) {
    f = move(funs.front()); // move the first lambda in f
    funs.pop_front(); // remove the element from deque //now the value is hold by f
  }
}
if (f) f(); //execute f
Casey
  • 41,449
  • 7
  • 95
  • 125
  • Hi Casey, probably (for now) the best solution is the first one because in the second case the lambdas will stored into heap and for performance reason could be better to use auto – Gian Lorenzo Meocci Jun 15 '14 at 21:07
  • 2
    @GianLorenzoMeocci Why should `auto` give better performance? `auto f` in the first snippet will be identical to `decltype(move(funs.front())) f`. So by declaring `decltype(move(funs.front())) f` in the second snippet will give the same. – Walter Jun 16 '14 at 07:55
  • because using auto you'll store the lambdas in stack – Gian Lorenzo Meocci Jun 16 '14 at 08:00