CODEWITHSUNDEEP
c++11c++14
David Simic
David Simic

Reputation: 2101

Declaring and starting a c++11 thread outside of main

I have a cpp file ("Plugin.cpp") which contains a small plugin that eventually gets linked into a much larger c++11 server project as a shared library.

It needs to start a thread and out of lazyness (it's a quick prototype) I simply declared and started it like so within Plugin.cpp:

#include <thread>

bool threadDone = false;

void myFunction() {
    while(!threadDone) {
    }
    // do stuff
}

std::thread myThread = std::thread(&myFunction);

// Called by server to stop tell plugin to cleanup.
void stopPlugin() {
    threadDone = true;
    myThreadDone.join();
    // ....
}

It seems like quite the anti-pattern to go starting threads as if the cpp file were a script file, but it works and doesn't seem to cause any problems when I run this code within the larger server project.

Without getting into the details of the larger project:

Can someone please detail what bad un-intended consequences may arise from doing this, as opposed to starting the thread in a more controlled manner from within an object or function?

My apologies if this is too obvious, I have not found this answer online and I have not coded in c++ recently so I am quite rusty. But it seems to me that something should eventually go very wrong.

Upvotes: 0

Views: 638

Answers (2)

Julius
Julius

Reputation: 1861

There are several issues with your example.

threadDone is shared data with a data race

threadDone is shared data (write from main thread, read from plugin thread). In your example there is no synchronization mechanism (such as std::mutex or std::atomic). Hence there is a data race in your example; yielding undefined behavior.

When dealing with threads in C++ you should know what the C++ memory model says about threads.

You can easily verify that the code is not thread safe by inspecting the assembly of the following code:

namespace buggy {

bool threadDone = false;
unsigned turn = 0u;

void myFunction() {
    while(!threadDone) {
        ++turn;
    }
}

}// buggy

which reads

buggy::myFunction():
        cmp     BYTE PTR buggy::threadDone[rip], 0
        jne     .L1
.L3:
        jmp     .L3
.L1:
        ret

Note that threadDone is compared exactly once. If it is false then there is an infinite loop (without any increment).

The easiest way to fix the above is given by the use of atomics:

#include <atomic>

namespace fixed {

std::atomic<bool> threadDone{false};
std::atomic<unsigned> turn{0u};

void myFunction() {
    while(!threadDone.load()) {
        turn.fetch_add(1u);
    }
}

}// fixed

which yields the assembly

fixed::myFunction():
        movzx   eax, BYTE PTR fixed::threadDone[rip]
        test    al, al
        jne     .L5
.L8:
        lock add        DWORD PTR fixed::turn[rip], 1
        movzx   eax, BYTE PTR fixed::threadDone[rip]
        test    al, al
        je      .L8
.L5:
        ret

Initialization order

I expect that the thread from your plugin uses global data as input and output. It is difficult to ensure that the data is properly initialized before use.

Lack of startPlugin, and stopPlugin may fail

This is just a note that the interface is asymmetric (startPlugin not available because the plugin always starts --- when?). In addition, stopPlugin fails if the thread is not joinable (e.g., if stopPlugin is called twice).

The latter may be a real issue or not.

Upvotes: 1

cplusplusrat
cplusplusrat

Reputation: 1445

I would have used an std::atomic<bool> instead of a bool threadDone. See When do I really need to use atomic<bool> instead of bool? for more details.

The way you are declaring your std::thread object, you do not have any direct control on at what point the thread is going to start. Is there a pressing need to declare the thread object this way? Another important consideration is if your shared library is loaded/linked against a separate target than the intended server, would it not spawn an additional unintended thread?

If you have a stopPlugin method which the server can call to stop the thread, you can always write a startPlugin method as well with the actual thread object nicely wrapped in a simple interface like below.

class Plugin {
public:
    void startPlugin();
    void stopPlugin();
private:
    std::thread myThread;
};

I very rarely use static storage the way you are using it even for much simpler structs and tend to prefer construct on first use idiom.

Upvotes: 3

Related Questions