C++ variadic factory with move semantics causes run-time crash

I've created a factory that's using variadic templates and auto-registration. I'm having a run-time crash with the code below with respect to move semantics when invoking a constructor (a parameter is moved) when trying to create a type that has auto-registered itself - via the Factory.

Could someone explain to me what the issue is causing the run-time crash and how to correct it?

I'm using c++17, so if there are constructs I should be using, I'd appreciate advice (example code) on that too.

EDIT: the 'access violation" crash occurs when ~Base() destructor executes - however, the member "_moveString" is NULL when the Base() constructor finishes. So, the memory clearly isn't being moved correctly in the constructor and then something causes a crash during destruction.

EDIT: Compiler versions tested are: Visual Studio 2017 with "Version 19.14.26430" and "GCC 7.3"

EDIT: removed code version where 'Base::_moveString' was a 'boost::optional' to 'std::unique_ptr'

EDIT: changed code to ensure exact function type match and only have a single param that we perform an "std::move" on (which is the focus of the issue)

#include <string>
#include <memory>
#include <typeindex>
#include <typeinfo>
#include <unordered_map>

using namespace std;

class Base
{
public:
    Base(unique_ptr<string>&& moveString)
        :
        _moveString(move(moveString)) // why is moveString empty here? Should have value "moveString"
    {
    }

    virtual ~Base() = default;
    virtual void DoSomething() const = 0;

protected:
    unique_ptr<string> _moveString;
};

class Factory final
{
public:
    template<typename My_Type, typename... Args>
    static unique_ptr<My_Type> Create(Args&&... args)
    {
        unique_ptr<My_Type> type = nullptr;
        auto iter = GetCreateFunctions().find(typeid(My_Type));

        if (GetCreateFunctions().end() != iter)
        {
            typedef unique_ptr<My_Type>(*create_func)(Args...);
            auto create = reinterpret_cast<create_func>(iter->second);
            //auto a = (get<1>(forward_as_tuple(forward<Args>(args)...))).get(); // DEBUGGING
            type = create(forward<Args>(args)...);
        }

        return type;
    }

    template<typename My_Type, typename Func>
    static bool Register(Func func)
    {
        bool isRegistered = false;

        if (GetCreateFunctions().end() == GetCreateFunctions().find(typeid(My_Type)))
        {
            GetCreateFunctions()[typeid(My_Type)] = reinterpret_cast<void*>(func);
            isRegistered = true;
        }

        return isRegistered;
    }

private:
    static unordered_map<type_index, void*>& GetCreateFunctions()
    {
        static unordered_map<type_index, void*> map;
        return map;
    }
};

class Derived final : public Base
{
public:
    Derived(unique_ptr<string>&& moveString)
        :
        Base(move(moveString))
    {
    }

    ~Derived() = default;

    void DoSomething() const override
    {
        if (_moveString)
        {
            // do something...
        }
    }

private:
    static const bool _isRegistered;

    static unique_ptr<Derived> Create(unique_ptr<string>&& moveString)
    {
        return make_unique<Derived>(move(moveString));
    }
};

const bool Derived::_isRegistered = Factory::template Register<Derived>(&Derived::Create);


int main(int argc, char** argv)
{
    string moveString = "moveString";
    unique_ptr<Base> myBase = Factory::template Create<Derived>(make_unique<string>(move(moveString)));

    if (myBase)
        printf("Success\n");

    return 0;
}

Solution

Your map stores void * and you reinterpret_cast function pointers to and from that. The code that consumes the function pointer is:

 // Args... is deduced from arguments to Create()
 typedef std::unique_ptr<MyType>(*create_func)(Args...);
 auto create = reinterpret_cast<create_func>(iter->second);

however the value you put into the map was the address of:

static std::unique_ptr<Derived> Create(unique_ptr<string>&& moveString)

This causes undefined behaviour. When calling a function through a function pointer, the called function must have exactly the same type as the function pointer points to. (Ref: C++17 [expr.call]/1).

The actual function type is unique_ptr<Derived>(unique_ptr<string>&&)
The type you cast to is pointer to unique_ptr<Derived>(unique_ptr<string>)

which is a mismatch on the parameter type.

When using a forwarding reference, Args is deduced to either T or T& (never T&&). The syntax Args&& produces either T& or T&&. So the required change is:

 typedef std::unique_ptr<MyType>(*create_func)(Args&&...);