11

In the following code:

smart pointer data member pImpl(class Impl) and raw pointer pc(class CAT) all are incomplete data type, there is no definition of these two classes in Widget.h

//widget.h

#ifndef W_H_
#define W_H_
#include <memory>

class Widget { 
    public:
        Widget();
        ~Widget() {    //
            delete pc; // I know I should put ~Widget to .cpp
                       // I just want to show the difference in behavior
                       // between raw pointer and smart pointer(both has incomplete type)
                       // when widget object destructs 
        }
    private:
        struct Impl; 
        std::shared_ptr<Impl> pImpl;  // use smart pointer
        struct CAT;
        CAT *pc;  //raw pointer
};
#endif

//widget.cpp

#include "widget.h"

#include <string>
#include <vector>
#include <iostream>
using namespace std;
struct Widget::CAT 
{
    std::string name;
    CAT(){cout<<"CAT"<<endl;}
    ~CAT(){cout<<"~CAT"<<endl;}
};      


struct Widget::Impl {
    std::string name;
    Impl(){cout<<"Impl"<<endl;}
    ~Impl(){cout<<"~Impl"<<endl;}
};


Widget::Widget()  
    : pImpl(std::make_shared<Impl>()),
      pc(new CAT)
{} 

//main.cpp

#include "widget.h"
int main()
{
    Widget w;
}

//output

Impl

CAT

~Impl

For the raw pointer data member, its destuctor is not called when widget object is destructed.

While the shared_ptr data member, its destructor has been correctly called.

To my understanding, in Widget::~Widget() it should generate some code automatically as the following:

        ~Widget() {
            delete pc; // wrote by me
            
            // generated by compiler
            delete pImpl->get();
        }

Why do shared_ptr data member and raw data member have different behavior when the widget gets destructed?

I test the code using g++4.8.2 in Linux

================================EDIT=============================== According to the answers, the reason is because of :

the code generated by compiler is NOT:

        ~Widget() {
            delete pc; // wrote by me
            
            // generated by compiler
            delete pImpl->get();
        }

it maybe something like:

        ~Widget() {
            delete pc; // wrote by me
            
            // generated by compiler
            pimpl.deleter(); //deleter will be initailized while pimpl object is initialized
        }
Community
  • 1
  • 1
camino
  • 10,085
  • 20
  • 64
  • 115
  • @JoachimPileborg I have called delete pc; in Widget::~Widget – camino Mar 15 '15 at 15:59
  • @JoachimPileborg Isn't it what he did in his dtor? – JBL Mar 15 '15 at 15:59
  • If you put the implementation of the destructor in the source file, does the behavior stay the same or is it correct? – Some programmer dude Mar 15 '15 at 16:01
  • Exactly the same code with a different arrangement [doesn't exhibit](https://ideone.com/Cny86Z) that behavior for me – JBL Mar 15 '15 at 16:06
  • @JoachimPileborg Both will works if put to source file. I am just wondering why smart_pointer works if not put ~Widget to source file. – camino Mar 15 '15 at 16:07
  • For this last question, the reason is that when an instance is destroyed, all the members have their destructor called. In the case of a smart pointer, its destructor actually deletes the pointer it holds. For a raw pointer, the "destructor" simply let go of the pointer, **but doesn't delete the memory it holds**. – JBL Mar 15 '15 at 16:09
  • @JBL Your codes works is because of that all definition is in one file. – camino Mar 15 '15 at 16:09
  • Yep, just saw the answer and it explains why apparently. – JBL Mar 15 '15 at 16:10
  • If you had used boost::checked_delete instead of delete (which you always should), you would have been warned about the problem. – Jon Kalb Mar 24 '15 at 00:38

3 Answers3

14

Because you forward declaration "CAT" in the header file, you have an incomplete data type. With this information the compiler fall into undefined behavior and the destructor may not be called.

What the standard says

if the object being deleted has incomplete class type at the point of deletion and the complete class has a non-trivial destructor or a deallocation function, the behavior is undefined.

Here you can find a detailed explanation: Why, really, deleting an incomplete type is undefined behaviour? Just moving the struct declaration to before the class definition should fix your problem:

struct CAT
{
    std::string name;
    CAT(){std::cout<<"CAT"<<std::endl;}
    ~CAT(){std::cout<<"~CAT"<<std::endl;}
};

class Widget {
    public:
        Widget();
        ~Widget() {
            delete pc; // I know we should put this code to cpp
                       // I am just want to show the difference behavior
                       // between raw pointer and smart pointer
                       // when widget object destruct
        }
    private:
        struct Impl;
        std::shared_ptr<Impl> pImpl;  // use smart pointer
        CAT *pc;  //raw pointer
};

And the output

Impl
CAT
~CAT
~Impl

Forward declarations are good to speed up compilation time, but can lead to problems when more information about the data type is needed.

But why does it work for smart pointers? Here's a better explanation: Deletion of pointer to incomplete type and smart pointers

Basically, the shared_ptr only needs the declaration when it initializes or resets the pointer. That means it doesn't need the complete type on the moment of the declaration.

This functionality isn't free: shared_ptr has to create and store a pointer to the deleter functor; typically this is done by storing the deleter as part of the block that stores the strong and weak reference counts or by having a pointer as part of that block that points to the deleter (since you can provide your own deleter).

Community
  • 1
  • 1
dfranca
  • 5,156
  • 2
  • 32
  • 60
5

Explanation

You are trying to delete an object of incomplete type, this is undefined behavior if the object type being deleted has a non-trivial destructor.

More about the matter can be read about in [expr.delete] in the Standard, as well as under the following link:


Note: The destructor of Widget::Cat is non-trivial since it is user-declared; in turn this means it is not called.



Solution

To fix the problem simply provide the definition of Widget::Cat so that it's not incomplete when you do delete pc.



Why does it work for shared_ptr?

The reason it works when using a shared_ptr is that the "point of deletion" doesn't happen until you actually construct the shared_ptr instance (through make_shared; ie. when the Deleter is actually instantiated.

Community
  • 1
  • 1
Filip Roséen - refp
  • 62,493
  • 20
  • 150
  • 196
0

A shared_ptr<T> is 3 and a half things.

It is a pointer to T, a Deleter, and a reference count. It is also a weak reference count (that is the half).

The Deleter tells the shared_ptr<T> what to do when the reference count hits 0. It is, in a sense, unrelated to the pointer to T: you can use what I call the "god mode" shared pointer constructor to completely divorce them -- shared_ptr<T>::shared_ptr( shared_ptr<U>, T* ) -- and get your reference count from the shared_ptr<U> and your pointer from the T*.

The point where the Deleter is bound is at construction: the two most common ways are via shared_ptr<T>::shared_ptr(T*) or via make_shared<T>. At that point, what happens when the reference count returns to 0 is fixed.

You can copy the shared_ptr<T> into a shared_ptr<Base> and the Deleter follows along. You can "god mode" steal the reference count, and pass a pointer to a member variable as the type pointed to: and the original Deleter follows along.

When a shared_ptr<T> is reaches 0 reference count, it has no idea what it will do to destroy: the Deleter is some arbitrary task at thd point of destruction, decided at the point of construction.

So if "how to destroy the T" was visible where the smart pointer was created, you are fine. In comparison, a call to delete ptr; directly needs thd "how to destroy T" to be visible at the point of deletion.

And that is why you get the different behaviour.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524