3
#include <iostream>
#include <string>
#include <thread>

using namespace std;

struct safe_thread : public thread
{
    using thread::thread;

    safe_thread& operator=(safe_thread&&) = default;

    ~safe_thread()
    {
        if (joinable())
        {
            join();
        }
    }
};

struct s
{
    safe_thread t;
    std::string text = "for whatever reason, this text will get corrupted";

    s() noexcept
    {
        std::cout << text << '\n'; // it works in constructor as expected
        t = safe_thread{ [this]
                         { long_task(); }};
    }

    void long_task()
    {
        for (int i = 0; i < 500; ++i)
        {
            std::cout << text << '\n'; // the text gets corrupted in here
        }
    }
};

int main()
{
    s s;
}

In the code above, the text variable would print correctly in the constructor. However, in the long_task() function running in a separate thread, the text gets corrupted (it outright crashes on another machine). How is that so? If the destructor of safe_thread would be run in the destructor of struct s, shouldn't the lifetime of thread and text last equally long? I.e they would both go out of scope when s goes out of scope at main()?

Liang Wang
  • 41
  • 3
  • Do you really want to make `s()` `noexcept`? The `std::thread` constructor can throw if the thread could not be started and `std::thread` can throw `std::bad_alloc` if the allocation fails. – walnut Dec 28 '19 at 09:37

1 Answers1

11

Your problem is in the order of declaration member variables inside s class.

int main()
{
    s s;
    // here is called dtor of S
}

When destructor is called data members are destroyed in reverse order of their declarations. You have:

safe_thread t; // [2]
std::string text = "for whatever reason, this text will get corrupted"; // [1]

so as first string [1] is destroyed, then [2] thread's destructor is invoked and during this invocation your program joins. But then it is accessing destroyed text variable. It is UB.

Change the order to be:

std::string text = "for whatever reason, this text will get corrupted";
safe_thread t;

With this approach, while joining t, text variable is still visible and not deleted.

rafix07
  • 20,001
  • 3
  • 20
  • 33