C++ Custom Thread Wrapper with Templatized Constructor causes compile time error for insertion into std::map

Question

I'm writing my own basic game engine and I'm multithreading it. I've created a RawThread object which wraps an std::thread with message queues, etc. I want to map these RawThreads to an id(unsigned int). So I try to insert one into an std::map<unsigned int, RawThread>, but I get a bunch of very ambiguous template errors. If I comment out the map.insert line, then the code compiles just fine, everything works, but I want to be able to insert a RawThread into the map, as tried in main().

I've stared at this code for an hour, checking to see if I got my RawThread constructor template wrong, or if I'm using my references/pointer incorrectly, but I can't see the problem.

#include <map>
#include <utility>
#include <thread>

class RawThread
{
public:

    RawThread(const RawThread& you)
    {
        *this = you;
    }



    ~RawThread()
    {
        delete mThread;
    }

    template<class Function, class... Args>
    RawThread(Function&&f, Args&&... args)
    {       
        mThread = new std::thread(std::forward<Function>(f),         
                                          std::forward<Args>(args)...);
    }

    const RawThread& operator=(const RawThread& in)
    {
        this->mThread = in.mThread;
        return *this;
    }

    void join()
    {
        mThread->join();
    }
    std::thread* mThread;
};



void hello(int x){while(true)++x;}

int main()
{
    int arg = 0;
    RawThread myThread(&hello, arg);
    unsigned int threadId = 0;
    std::map<unsigned int, RawThread> threadMap;
    threadMap.insert(std::make_pair(threadId, myThread));
    return 0;
}

I got a nasty looking error, so I put it on pastebin: https://pastebin.com/9A4wN7kL

Answer 1

The problem with insert is that a copy of RawThread is made by calling

template<class Function, class... Args>
RawThread(Function&&f, Args&&... args)

then Function is instance of RawThread and Args is empty. std::thread constructor can take instance of object as its first parameter but then the class of this instance must provide operator()() as the body of thread. Your class is missing that. You can add it and the code will compile.

class RawThread {
    //....
    void operator()() {}
};

But it is not what you want.

---

For me you should implement RawThread in the way that only one instance of this class keeps active mThread pointer. RawThread should be only moved, copies should be disabled. In case when many instances share the same mThread value, which one should call join or delete this object?

Version which supports only move may be:

#include <map>
#include <utility>
#include <thread>

class RawThread {
public:
    ~RawThread()
    {
        if (mThread)
        {
            if (mThread->joinable())
                mThread->join();
            delete mThread;
        }
    }

    RawThread(RawThread&& theOther) {
        mThread = theOther.mThread;
        theOther.mThread = nullptr;
    }

    RawThread& operator=(RawThread&& theOther) {
        mThread = theOther.mThread;
        theOther.mThread = nullptr;
        return *this;        
    }

    template<class Function, class... Args>
    RawThread(Function&&f, Args&&... args) {
        mThread = new std::thread(std::forward<Function>(f),         
                                  std::forward<Args>(args)...);
    }

    void join() {
        mThread->join();
    }
    std::thread* mThread;
};

void hello(int x){while(true)++x;}

int main()
{
    int arg = 0;
    RawThread myThread(&hello, arg);
    unsigned int threadId = 0;
    std::map<unsigned int, RawThread> threadMap;
    threadMap.emplace(threadId,std::move(myThread)); // move object into map
    return 0;
}