Initialization of data member thread and mutex. Does wrong order has Undefined Behavior?

user_2805305 0
Viewed 649

I stumbled upon what I think is a very easy way to unknowingly shoot yourself in the foot.

first a little introduction

The initialization order of data members is the order of declaration of the data members. So this is illegal:

struct A
{
    std::size_t i_;
    std::size_t length_;

    A(std::size_t length)
      : i_{length_} // UB here. `length_` is uninitialized
        length_{length}
    {}
};

Because data member length_ is uninitialized when used in the initializer of i_. Fortunately both gcc and clang give a very nice warning for this. The simple solution is to initialize each data members from the parameters, i.e. i_{length}.

now to the main point

But how about when it is not immediately obvious. E.g. when a data member is a std::thread

struct X
{
    std::thread thread_;
    std::mutex mutex_;

    X() : thread_{&X::worker_thread, this}
    {}

    auto worker_thread() -> void
    {
        // use mutex_  
        std::lock_guard lk{mutex_}; // boom?
        // ..
    }
};

The same situation arises when using data member initializer:

struct X
{
    std::thread thread_{&X::worker_thread, this};
    std::mutex mutex_;
};

This looks very innocent and neither gcc and clang warn on this scenario. This is not surprising, as the dependency is hidden.

I would immagine the above scenario is not uncommon so I am looking on confirmation that this is indeed UB. And declare the std::mutex data member last, or default initialize it and assign it later.

multithreading c++
2 Answers

Yes, it is indeed undefined behavior. As a matter of fact, you have over-complicated the example with threads and mutexes. Every time you are using this in initialization of members (explicitly or implicitly), you are opening yourself to troubles. Easier example:

struct A {
    int y;
    int x = 0;
    A() : y(sety()) { }

    int sety() { return x; } // Ka-boom!
};

It is always quite dangerous to call non-static member functions from within member initialization; and one generally has also to be careful when calling member functions from constructor body.

user_52450330

There are two possible UB:

  • if the calling of the member function of mutex_ here is UB.
  • if the initialization of mutex_ and the accessing of it lead to a problematic data race.

The calling of the member function will lead to UB if,

  1. The construction of std::thread is sequenced before the construction of std::mutex, and
  2. std::mutex isn't trivially constructible.

15.7.1 For an object with a non-trivial constructor, referring to any non-static member or base class of the object before the constructor begins execution results in undefined behavior.

33.3.2.2 thread constructors [...] 6. Synchronization: The completion of the invocation of the constructor synchronizes with the beginning of the invocation of the copy of f.

33.4.3.2.3 The mutex types shall be DefaultConstructible and Destructible.

The initialization of mutex_ is sequenced after the initialization of std::thread (since they are data member), which is synchronized with the beginning of the thread. And if std::mutex isn't trivial constructible(This is unspecified). Then this will lead to an potential UB because of accessing an object before its construction. Given that the calling of member function and the initialization is potentially concurrent.

For data race:

6.8.2.1 Two expression evaluations conflict if one of them modifies a memory location (6.6.1) and the other one reads or modifies the same memory location.

6.8.2.1.20 The execution of a program contains a data race if it contains two potentially concurrent conflicting actions, at least one of which is not atomic, and neither happens before the other, except for the special case for signal handlers described below. Any such data race results in undefined behavior.

There is a good chance that the construction of std::mutex will modify some memory location which need to be modified by std::mutex::lock, but there is also good chance that such modification is somehow atomic. But they are not specified by the standard.

As a conclusion, I think that whether such usage lead to undefined behavior is unspecified.

user_70384460
Related