Matthias Brandt
Matthias Brandt

Reputation: 352

Static map used before it is initialized

I'm trying to implement a factory pattern like this.

The problem right now is that the program terminates with a segfault in the register function because the map is not initialized yet.

// initialise the registered names map
std::map<std::string, factoryMethod> SourceFactory::registeredClasses_ = { };

bool SourceFactory::Register(std::string name, factoryMethod createMethod) {
    // registeredClasses_ = { }; // This prevents the segfault but does not work for obvious reasons
    auto temp = std::make_pair(name.c_str(), createMethod);

    std::pair<std::map<std::string, factoryMethod>::iterator, bool> registeredPair =
            SourceFactory::registeredClasses_.insert(temp);

    return registeredPair.second;
}

Why is it possible for Register() to be called before the initialization of the map? I tried initializing the map in the header file but then I get the linker error

multiple definition of SourceFactory::registeredClasses_

A solution would be to set a static bool isInitialized=false and then initialize the map accordingly. But I hope this can be avoided.

Upvotes: 0

Views: 112

Answers (2)

Lightness Races in Orbit
Lightness Races in Orbit

Reputation: 385144

This is a common problem known as the static initialisation order fiasco.

Objects with static storage duration at namespace scope are constructed in-order within a translation unit, but you don't know whether those in other translation units may undergo initialisation first.

Instead of having your container at namespace scope, make it function static (perhaps returned from a new GetRegistry() function?) so that it is constructed on first use. That could be use from main, use from initialisation of another static-duration "thing" (which is presumably where your Register call is coming from), use from the moon…

This is also why the proper way to write a singleton is to have a GetInstance() function that declares (staticly!) the instance within the function's scope.

A solution would be to set a static bool isInitialized=false and then initialize the map accordingly. But I hope this can be avoided.

Nope. Not only would you then have the same problem with the isInitialized flag, but there's nothing you can "do" with that information. You can't "initialize" something other than in an initializer, and the whole problem is that the initializer hasn't been used yet. You could assign to the map, but that would have undefined behaviour because you'd be assigning to something that doesn't exist yet… and then it would be initialized later anyway!

Upvotes: 0

molbdnilo
molbdnilo

Reputation: 66371

It's possible when Register is called from a different translation unit before the registry has been initialised.
Unfortunately, adding a static flag would not solve anything because that would not be initialised either.

A convenient solution is to add a level of indirection:

// static
std::map<std::string, factoryMethod>& SourceFactory::registry()
{
    static std::map<std::string, factoryMethod> registeredClasses;
    return registeredClasses;
}

bool SourceFactory::Register(const std::string& name, factoryMethod createMethod) {
    auto temp = std::make_pair(name, createMethod);
    return registry().insert(temp).second;
}

Upvotes: 1

Related Questions