Safe placement new & explicit destructor call

Question

This is an example of my codes:

template <typename T> struct MyStruct {
    T object;
}

template <typename T> class MyClass {
    MyStruct<T>* structPool;
    size_t structCount;

    MyClass(size_t count) {
        this->structCount = count;
        this->structPool  = new MyStruct<T>[count];
        for( size_t i=0 ; i<count ; i++ ) {
            //placement new to call constructor
            new (&this->structPool[i].object) T(); 
        }
    }

    ~MyClass() {
        for( size_t i=0 ; i<this->structCount ; i++ ) {
            //explicit destructor call
            this->structPool[i].object.~T(); 
        }
        delete[] this->structPool;
    }
}

My question is, is this a safe way to do? Do I make some hidden mistake at some condition? Will it work for every type of object (POD and non-POD)?

Answer 1

No, because both your constructor and destructor are invoked twice. Because you have this:

template <typename T> struct MyStruct {
    T object;
}

When you construct a MyStruct<T> the compile will construct the inner T and when you delete the object the inner T will have the destructor called automatically.

For this example, there is no need for placement new or an explicit destructor call.

Placement new would be useful if you allocate raw memory. For example, if you changed your new to:

this->structPool  = new char[sizeof(T) * count];

then you would want to placement new and explict destructor call.

Answer 2

No, it is certainly not even remotely safe way to do it. When you do new MyStruct<T>[count] for non-POD T, each MyStruct<T> object in the array already gets default-constructed, meaning that the constructor for the object member gets called automatically. Then you attempt to perform an in-place construction (by value-initialization) on top of that. The resultant behavior is undefined.

The same problem exists with the deletion.

What is it you are trying to achieve? Just do new MyStruct<T>[count]() (note the extra empty ()) and it will already perform value-initialization for each element of the array (exactly what you are trying to do "manually" afterwards). Why do you feel you have to do it by in-place construction?

Likewise, when you do

delete[] this->structPool;

it automatically calls the destructor for each MyStruct<T>::object member in the array. No need to do it manually.

Answer 3

1. Remembering new will always call the constructor, no matter if it is placement or not.

-- So your code used new twice. This will call the constructor twice. If you want to avoid this, either:

Change your first new into malloc(or any kind of alloc)

Remove your second placement new

1. To delete the objects in an array, the best way is: Invoke every object's destructor; free the memory.

-- So you can do either:

Delete the object with delete[] if you are using new[]

Call every destructor and do a C style free if you are using malloc and placement new

Answer 4

template <typename T> class MyClass {
    void* structPool;
    size_t structCount;

    MyClass(size_t count)
      : structPool(new char[sizeof(T)*count], structCount(count)
    {
        //placement new to call constructor
        for( size_t i=0 ; i<count ; i++ )
            new (structPool+i*sizeof(T)) T(); 
    }

    ~MyClass() {
        //explicit destructor call
        for( size_t i=0 ; i<structCount ; i++ )
            reinterpret_cast<T*>(structPool+i*sizeof(T))->~T(); 
        delete[] structPool;
    }
}

Note that this isn't thread-safe: If one of the constructors causes an exception, it doesn't call the destructors on the objects already constructed and it leaks the memory. When one of the destructors throws, this fails, too.

Have a look at std::vector in your favorite std lib implementation in order to see how to do this right. However, that leads to the question: Why do you want to do this in the first place?
A std::vector already does all this, does it right, you can use it out of the box, and everyone looking at your code will understand it immediately:

template <typename T> class MyClass {
    std::vector<T> data_;
    MyClass(size_t count) : data() {data.resize(count);}
    //~MyClass() not needed
}