1

I was trying an auto-registering CRTP factory class. My aim is not using macro to register the class types and without calling explicitely a register method.

I did my try basing on this answer:

factory.h

template<typename Base>
struct Factory{
    static Base* create(const QString& name){
        auto callable = map.value(name);
        return callable();
    }

    template<typename Derived>
    static void subscribe(QString name){
        // insert already-exist-check here
        qDebug() << "registered: " << name;
        map.insert(std::move(name), [](){return new Derived{};});
    }

    static QMap< QString, std::function<Base*(void)> > map;
};

template<typename Base>
QMap< QString, std::function<Base*(void)> > Factory<Base>::map;


template<typename Base, typename Derived>
class Registrable{
protected:
    virtual QString registerName() const = 0;

    Registrable(){
        isRegistered;
    }
    ~Registrable() = default;

    static bool init(){
        Derived t;
        Factory<Base>::template subscribe<Derived>(t.registerName());
        return true;
    }

private:
    static bool isRegistered;
};

template<typename Base, typename Derived>
bool Registrable<Base, Derived>::isRegistered = Registrable<Base, Derived>::init();

main.cpp

struct MyFactoryBase{
    virtual ~MyFactoryBase() = default;
    virtual void method() const = 0;
};

struct MyFactory1 : public MyFactoryBase, public Registrable<MyFactoryBase, MyFactory1>{
    QString registerName() const override{
        return "Class1";
    }
    void method() const override{ qDebug() << "yay Class1"; }

    MyFactory1() : MyFactoryBase(), Registrable<MyFactoryBase, MyFactory1>(){}
};

struct MyFactory2 : public MyFactoryBase, public Registrable<MyFactoryBase, MyFactory2>{
    QString registerName() const override{
        return "Class2";
    }
    void method() const override{ qDebug() << "yay Class2"; }

    MyFactory2() : MyFactoryBase(), Registrable<MyFactoryBase, MyFactory2>(){}
};

int main(){
//    auto* fac = Factory<MyFactoryBase>::create("Class1");
//    auto* fac2 = Factory<MyFactoryBase>::create("Class2");
}

it crash with a segfault when trying to use map.insert() during the first subscribe. What I'm missing?

Moia
  • 2,216
  • 1
  • 12
  • 34
  • 1
    And what's the error? Did you debug your code? – Matthieu Brucher Feb 11 '19 at 11:57
  • @MatthieuBrucher debug segfault right after the qDebug inside the subscribe method, when calling map.insert() – Moia Feb 11 '19 at 12:01
  • Kind of sounds like static inititalization order fiasco to me, although I have no idea whether that's the actual problem. – Max Langhof Feb 11 '19 at 12:04
  • 1
    You are a victim of [static initialization order fiasco](https://isocpp.org/wiki/faq/ctors#static-init-order); you can't be sure if `static QMap map` is initialized before `static bool isRegistered`, which inserts into `map`. – rustyx Feb 11 '19 at 14:04

1 Answers1

2

As suggested in comments, it was a static initialization order fiasco.

I solved it like suggested in ISOCpp's How do I prevent the “static initialization order problem” for my static data members?

In any event, it’s always portable and safe to change the X::x_ static data member into a static member function:

So changing

static QMap< QString, std::function<Base*(void)> > map;

to

   static QMap< QString, std::function<Base*()> >& map(){
        static QMap< QString, std::function<Base*()> > map;
        return map;
    }

and updating all the calls from map with map() fix the issue.

int main(){
    auto* fac = Factory<MyFactoryBase>::create("Class1");
    auto* fac2 = Factory<MyFactoryBase>::create("Class2");

    fac->method();
    fac2->method();

    return 0;
}

output:

registered: Class1
registered: Class2
yay Class1
yay Class2
Moia
  • 2,216
  • 1
  • 12
  • 34
  • Did you read the very next Q&A in the FAQ? It's also possible some code might attempt to `create` an object after your function-local static object has already been destroyed. – aschepler Feb 12 '19 at 14:34
  • @aschepler could you please explain what you mean? – Moia Feb 12 '19 at 14:39