Globally called constructor of a class template that adds this pointer to static list causes error

Question

I have a class template that keeps track of all instances of a specific instance of the class template with a list of pointers. In the constructor, I push_back(this) to add the created instance to the class. If I create an instance within main, it works fine. But if I create a global instance, the constructor causes an error. The error is from the list.push_back() and it says that the insertion iterator is out of range of the container. I have recreated the bug with minimal code.

Header file

#ifndef HEADER_H
#define HEADER_H

#include <iostream>
#include <list>

template<typename T>
class A
{
public:
    static std::list<A<T>*> l;

    A( T t ) : val( t ) { l.push_back( this ); }
    ~A() { l.remove( this ); }

    T val;

    void print() { std::cout << val;  }
};

template<typename T>
std::list<A<T>*> A<T>::l;

#endif

Source file

#include "Header.h"

A<int> temp( 0 );

void main()
{
    temp.print();

    char c;
    std::cin >> c;
}

I'm guessing it has something to do with the constructor call order, but does anyone know how to fix this?

There are some arcane rules related to static initialization order fiasco and template instantiation order that are harder to wrangle than a herd of cats. The easiest solution here is the function static scope trick. Declare the statically-scoped objects inside a function that returns a reference to it. — Sam Varshavchik, Mar 07 '20 at 02:24

walnut · Accepted Answer · 2020-03-07T03:04:05.083

Static members of template class specializations have unordered dynamic initialization, meaning that there is no guarantee in what order they will be initialized relative to other dynamic initialization.

If A<int>::l is initialized after temp, then initialization of temp will try to access A<int>::l before its lifetime began, causing undefined behavior.

The usual solution to this is to put the static variable in a static function and call that instead. Local static variables are initialized when execution reaches their declaration the first time, so it is always properly ordered.

template<typename T>
class A
{
public:
    static auto& l() {
        static std::list<A<T>*> instance;
        return instance;
    }

    A( T t ) : val( t ) { l().push_back( this ); }
    ~A() { l().remove( this ); }

    T val;

    void print() { std::cout << val;  }
};

Non-const global variables should however be avoided as much as possible in the first place. They usually make reasoning about the code more complex and have problems such as initialization order here.

I don't know what the purpose of the instance list here is, but it has similar issues and should probably be avoided if possible. Additionally it will have quite bad performance, especially if you have many instances of A<T> around, because l.remove( this ); will take linear time in the number of elements in the list. Consider using std::set or std::unordered_set instead of std::list.

Additional notes:

The return type of main must be int in C++. void as return type is a non-standard extension of some compilers.

Your class is violating the rule of 0/3/5. You need to define copy (and move) constructor and assignment operator with correct semantics, otherwise your instance list will not contain all instances when your class is copied.

You also might want to save const A<T>* instead of A<T>* in the list. The latter is dangerous. If you ever declare a variable of type const A<T>, then the pointer you save in the list will still be non-const. If you then use the pointer from the list to modify the object, you will not be warned that you are trying to modify a const object (which causes undefined behavior).

score 0 · Answer 2 · answered Mar 15 '20 at 10:39

Using a block scoped static variable (local static variable in a function / static function) is very good approach because initialization of class template static data members and other static/thread-local variables are indeterminately sequenced with respect to each other and this causing problem of accessing a variable which is not yet initialized. read more about it, at Static non-local initialization

But this approach will not allow to use A<T>::l, because then, std::list<A<T>*> l variable is not static member of class A<T>, It means if static member of a class can't be avoided due to some requirements then this unordered initialization problem has to be resolved.

Now unordered initialization can be converted into ordered initialization by using explicit template initialization, as follows,

template class A<int>; //explicit template initialization

A<int> temp(1);

It will make sure that std::list<A<T>*> l get initialized before temp object initialization. This explicit template initialization has to be done in each translation unit where you want to create non-local variable of A<T>. Let's look at the code,

Header file is similar (no change),

Source file

#include "Header.h"

template class A<int>; //explicit template initialization

A<int> temp(0);

int main(int , char *[]){
    std::cout<< "temp.val = ";

    temp.print();

    std::cout<< '\n';
}

Globally called constructor of a class template that adds this pointer to static list causes error

2 Answers2