Using the Nifty Counter C ++ Idiom, should the constructor be called twice?

advertisements

When trying to initialize static members with the Nifty Counter C++ Idiom I got some troubles with it. Can you explain how to correctly use the ideom in the following case?

The problem seems to be following: I have two static objects in different compilation units, where the one is using the static members of the other (CDataFile). Because the initialization order in this case is not defined – and in the production environment it was in the wrong order –, I tried to use the Nifty Counter idiom. But it seems, that now the static members of CDataFile are initialized twice (constructor called twice). First time, the constructor gets called in the CDataFileInitializer which is fine. After that the static members are used (mSome gets filled), but then the constructor of CSomeClass is called a second time and the content of mSome is cleared.

// datafile.h
class CDataFile
{
    friend class CDataFileInitializer;
protected:
    static CSomeClass mSome;
    // other code
};

static class CDataFileInitializer
{
public:
    CDataFileInitializer();
} dfinitializer;

// datafile.cpp
static int nifty_counter;
CSomeClass CDataFile::mSome; // second initialization comes from here?

CDataFileInitializer::CDataFileInitializer()
{
    if (!nifty_counter++)
    {
        printf("CDataFileInitializer Constructor\n");
        CDataFile::mSome= CSomeClass(); // first initialization
    }
}


The line:

CSomeClass CDataFile::mSome;

defines a variable of type CSomeClass. The initialisation of this variable happens in two stages: First it is zero-initialised, this (approximately) means that the memory in which it resides is all set to 0. After that, dynamic-initialisation occurs. This causes its constructor to be run.

dfinitializer follows a similar "zero-initialise then dynamic initialise" pattern. In its dynamic initialisation step it calls operator= on CDataFile::mSome, in order to assign a new default constructed CSomeClass() to mSome.

This step is totally pointless, because the dynamic initialisations of mSome and of dfinitializer are indeterminately sequenced relative to each other. If dfinialiser gets initialised first, it will attempt to assign to an object which has not been created (and which will later be default-constructed), and if it gets initialised second it will reassign to an object which has already been created.

Instead of:

CSomeClass CDataFile::mSome;

you should create a region of storage in which the object can be constructed:

alignas(CSomeClass) unsigned char CDataFile::mSome[sizeof(CSomeClass)];

Then change CDataFileInitializer to:

CDataFileInitializer::CDataFileInitializer()
{
    if (!nifty_counter++)
    {
        printf("CDataFileInitializer Constructor\n");
        new (&CDataFile::mSome) CSomeClass();
    }
}

An alternative would be to use a function static variable:

CSomeClass& getMSome() {
    static CSomeClass mSome;
    return mSome;
}

This will lazily initialise mSome in a thread-safe manner.