Using a shared pointer in a std::thread

Question

I have a scenario where:

1. I launch a new thread from within a dll that does some work.

2. The dlls destructor could be called before the new thread finishes its work.

3. If so I want to set a boolean flag in the destructor to tell the thread to return and not continue.

If I try the following then I find that because the destructor is called and MyDll goes out of scope then m_cancel is deleted and its value is unreliable (Sometimes false, sometimes true) so I cannot use this method.

Method 1

//member variable declared in header file
bool m_cancel = false;
MyDll:~MyDll()
{
    m_cancel = true;
}

//Function to start receiving data asynchronously  
void MyDll::GetDataSync()
{
    std::thread([&]() 
    {
        SomeFunctionThatCouldTakeAWhile();

        if( m_cancel == true )
          return;

        SomeFunctionThatDoesSomethingElse();    
    }
}

So I have looked at this example Replacing std::async with own version but where should std::promise live? where a shared pointer is used which can be accessed from both threads.

So I was thinking that I should:

1. Create a shared pointer to a bool and pass it to the new thread that I have kicked off.

2. In the destructor, change the value of this shared pointer and check it in the new thread.

Here is what I have come up with but I'm not sure if this is the proper way to solve this problem.

Method 2

//member variable declared in header file
std::shared_ptr<bool> m_Cancel;

//Constructor
 MyDll:MyDll()
{
    m_Cancel = make_shared<bool>(false);
}
//Destructor
MyDll:~MyDll()
{
    std::shared_ptr<bool> m_cancelTrue = make_shared<bool>(true);

    m_Cancel = std::move(m_cancelTrue);
}

//Function to start receiving data asynchronously  
void MyDll::GetDataSync()
{
    std::thread([&]() 
    {
        SomeFunctionThatCouldTakeAWhile();

        if( *m_Cancel.get() == true )
            return;

        SomeFunctionThatDoesSomethingElse();    
    }
}

If I do the above then the if( *m_Cancel.get() == true ) causes a crash (Access violation)

1. Do I pass the shared pointer by value or by reference to the std::thread??

2. Because its a shared pointer, will the copy that the std::thread had still be valid even MyDll goes out of scope??

How can I do this??

Method 3

//Members declared in header file
std::shared_ptr<std::atomic<bool>> m_Cancel;

//Constructor
 MyDll:MyDll()
{
    //Initialise m_Cancel to false
    m_Cancel = make_shared<std::atomic<bool>>(false);
}
//Destructor
MyDll:~MyDll()
{
    //Set m_Cancel to true
    std::shared_ptr<std::atomic<bool>> m_cancelTrue = make_shared<std::atomic<bool>>(true);

    m_Cancel = std::move(m_cancelTrue);
}

//Function to start receiving data asynchronously  
void MyDll::GetDataSync()
{
    std::thread([=]() //Pass variables by value
    {
        SomeFunctionThatCouldTakeAWhile();

        if( *m_Cancel.get() == true )
            return;

        SomeFunctionThatDoesSomethingElse();    
    }
}

What I fund is that when the destructor gets called and then if( *m_Cancel.get() == true ) is called it always crashes.

Am I doing something wrong??

Solution

I have added in a mutex to protect against the dtor returning after cancel has been checked in the new thread.

//Members declared in header file
std::shared_ptr<std::atomic<bool>> m_Cancel;
std::shared_ptr<std::mutex> m_sharedMutex;

//Constructor
 MyDll:MyDll()
{
    //Initialise m_Cancel to false
    m_Cancel = make_shared<std::atomic<bool>>(false);
    m_sharedMutex = make_shared<std::mutex>();
}
//Destructor
MyDll:~MyDll()
{
    //Set m_Cancel to true
    std::shared_ptr<std::atomic<bool>> m_cancelTrue = make_shared<std::atomic<bool>>(true);

    std::lock_guard<std::mutex> lock(*m_sharedMutex);//lock access to m_Cancel
    {
        *m_Cancel = std::move(cancelTrue);
    }
}

//Function to start receiving data asynchronously  
void MyDll::GetDataSync()
{
    auto cancel = this->m_Cancel;
    auto mutex = this->m_sharedMutex;

    std::thread([=]() //Pass variables by value
    {
        SomeFunctionThatCouldTakeAWhile();

        std::lock_guard<std::mutex> lock(*mutex);//lock access to cancel
        {
            if( *cancel.get() == true )
                return;

            SomeFunctionThatDoesSomethingElse();    
        }
    }
}

Answer 1

Step 2 is just wrong. That's a design fault.

Your first mechanism doesn't work for a simple reason. m_cancel==false may be optimized out by the compiler. When the destructor returns, m_cancel ceases to exist, and no statement in the destructor depends on that write. After the destructor returns, it would be Undefined Behavior to access the memory which previously held m_cancel.

The second mechanism (global) fails for a more complex reason. There's the obvious problem that you have only one global m_Cancel (BTW, m_ is a really misleading prefix for something that's not a member). But assuming you only have one MyDll, it can still fail for threading reasons. What you wanted was not a shared_ptr but a std::atomic<bool>. That is safe for access from multiple threads

[edit] And your third mechanism fails because [=] captures names from the enclosing scope. m_Cancel isn't in that scope, but this is. You don't want a copy of this for the thread though, because this will be destroyed. Solution: auto cancel = this->m_Cancel; std::thread([cancel](...

[edit 2] I think you really should read up on basics. In the dtor of version 3, you indeed change the value of m_cancel. That is to say, you change the pointer. You should have changed *m_cancel, i.e. what it points to. As I pointed out above, the thread has a copy of the pointer. If you change the original pointer, the thread will continue to point to the old value. (This is unrelated to smart pointers, dumb pointers behave the same).