c++ - lock_guard initialization in if-initializer instead of simple scope

Question

Is it correct that

if (std::lock_guard lk(m_state.mtx); true) {
  m_state.aborting = true;
}

and

{
  std::lock_guard lk(m_state.mtx)
  m_state.aborting = true;
}

are 100% identical?

---

Here is the background on my question: In Mastering the C++17 STL, the author gives a code example for a thread pool. It contains the following block (page 208):

~ThreadPool() {
  if (std::lock_guard lk(m_state.mtx); true) {  // <<--- Line of interest
    m_state.aborting = true;
  }

  m_cv.notify_all();
  for (std::thread& t : m_workers) { 
    t.join();
  }
}

The marked line uses the initializer part of an if-statement to create the lock_guard. This has been used in other questions in order to limit the scope of the lock_guard to the if-statement while also protecting the evaluation of the if-statement itself.

What confuses me in this example is that the if statement has no proper condition. According to the cppreference page on if,

If init-statement is used, the if statement is equivalent to

{
  init_statement
  if ( condition )
    statement-true
}

In this case, this would mean that if (std::lock_guard lk(m_state.mtx); true) {...} is equivalent to

{
  std::lock_guard lk(m_state.mtx);
  if (true) {
    m_state.aborting = true;
  }
}

Am I missing either a case where this is not 100% identical or a good reason why it has been written with an if-statement?

The same pattern is used in a different example on the next page, so I think that it was done on purpose and not a leftover from when there was an actual condition.

question from:https://stackoverflow.com/questions/65644053/lock-guard-initialization-in-if-initializer-instead-of-simple-scope

Answer 1

I'm the author of Mastering the C++17 STL.

I'd already forgotten that construct, so there can't have been any important reason I used it. I believe Igor is correct: the choice would have been between

~ThreadPool() {
  if (std::lock_guard lk(m_state.mtx); true) {
    m_state.aborting = true;
  }
  m_cv.notify_all();
  for (std::thread& t : m_workers) {
    t.join();
  }
}

and something such as

~ThreadPool() {
  if (true) {
    std::lock_guard lk(m_state.mtx);
    m_state.aborting = true;
  }
  m_cv.notify_all();
  for (std::thread& t : m_workers) {
    t.join();
  }
}

which does the same thing (arguably clearer) but takes one extra line of code in the physical book.

Personally I don't like to write just { to open a block; I would always rather write if (true) {. This helps the reader understand that the if wasn't accidentally omitted (or merge-conflicted away); the programmer really intends the block to run always. If you cuddle all your braces all the time, as I (often) do, then it also maintains the invariant that you never have a { or } sitting alone on a line — for whatever that might be worth.

The other thing going on there is that it's specifically in the context of notifying on a condition variable. I definitely used to write

std::lock_guard lk(m_state.mtx);
m_state.aborting = true;
m_cv.notify_all();

with the notify happening inside the scope of the lock; over a period of years, I heard "you should release the lock before notifying" often enough that eventually I also started saying it (even though I still have no good intuition for why that should be the case). This was actually the subject of my fourth-ever SO question, five years before I wrote the book: C++11 std::condition_variable: can we pass our lock directly to the notified thread?

So I may have been distracted (underthinking) and/or self-conscious (overthinking) while writing this particular snippet, which might have led me to try out this particular weird construct.

The example on the next page is actually the condition-variable thing exactly:

void enqueue_task(UniqueFunction task) {
  if (std::lock_guard lk(m_state.mtx); true) {
    m_state.work_queue.push(std::move(task));
  }
  m_cv.notify_one();
}

Thread-safety-wise, this is exactly equivalent to

void enqueue_task(UniqueFunction task) {
  std::lock_guard lk(m_state.mtx);
  m_state.work_queue.push(std::move(task));
  m_cv.notify_one();
}

except that people kept telling me not to put the notify inside the lock. So the if (...; true) version grudgingly satisfies that popular wisdom while spending only one line of code instead of two.

It's even possible that I had originally notified under the lock, and then the book's one technical reviewer said "don't notify under the lock," and then I made this late-breaking change specifically to deal with that in the minimal number of lines. I don't remember whether that was the actual story, but it's quite plausible to me.