Moving objects can change the order destructors are called. Why isn't that a problem?

Question

In the following example, a mutex is used to protect a file:

#include <fstream>
#include <memory>
#include <mutex>

std::mutex m;

int main() {
    std::unique_ptr<std::lock_guard<std::mutex>> ptr_1 = std::make_unique<std::lock_guard<std::mutex>>(m);
    std::fstream file_1("file_name.txt");
    std::unique_ptr<std::lock_guard<std::mutex>> ptr_2{std::move(ptr_1)};
}

When the destructors are called, the mutex will be unlocked before the file is closed. This can cause a race condition. It seems to me that there must be some guideline about move-operations, destructors, or ownership that I don't know about. What design guideline has been broken?

Answer 1

There are two guidelines/best practices that I see are violated here.

The first is enshrined in the C++ Core Guidelines as R.5:

Prefer scoped objects, don’t heap-allocate unnecessarily

Using heap allocation removes the structure afforded by the lexical scoping rules in C++. It is effectively an assertion that the programmer will manage lifetimes.

If you do not heap-allocate, the code doesn't compile:

std::lock_guard<std::mutex> lg_1{m};
std::fstream file_1("file_name.txt");
std::lock_guard<std::mutex> lg_2{std::move(lg_1)};

The second is CP.50:

Define a mutex together with the data it guards

The spirit of that rule is to encapsulate the synchronization. Put the data and lock together, and expose an API that is more difficult to misuse. Many designs would still have an RAII guard, because that is a flexible option, so you still have to put that on the stack. A guard type isn't strictly necessary though.

Answer 2

In general, C++ programmers should be aware that different C++ objects have different lifetimes and get destroyed at different points in time. If you have an object that holds some special items (e.g. mutexes, file handles, sockets) and you have rules about the required lifetime of those items, then it's up to the programmer to know those rules and consider them before they move items from one object to another.

I wouldn't say any design rule was broken in your example, I would just say the programmer introduced a bug when they added the move operation without thinking about the obvious implications of that.

Answer 3

A mutex helps you do multithread programming and, in such a case, you don't have the same stack frame and you don't usually care about any sort of destruction order. When using multiple threads, the order depends on the OS, not the compiler.

A mutex is not the sort of the object you want to own (at the C++ class level), copy or move. You take ownership at the OS level, not at the C++ level and it has no meaning in a single thread (i.e. singe stack frame where the order matters).

It's similar to when multiple threads are waiting for the mutex, you don't know the order that the threads will be released and you shouldn't code based on that.