placement new on shared_ptr make seg fault when delete

Question

Edit Solution::

In fact, i juste forget the placment new in the copy constructor ><"

Question:

I have a weird problem. After having tried for a long momnet origin I found masi does not understand. If someone can explain to me why.

My class:

class B; //on other file
class A {
   public:
     A(int type) : type(type)
     {
        switch(type)
        {
           case TOKEN:
           {
             for(int i=0;i<4;++i)
                new(&token.h[i].link) shared_ptr<B>; //< init the ptr on the addr (because of union)
           }break;
           case OTHER: {}break;
        }
     }
     ~A()
      {
        switch(type)
        {
            case TOKEN:
            {
             for(int i=0;i<4;++i)
             {
                /*option 1*/ token.h[i].link.~shared_pt<B>(); //< Make seg fault
               /*option 2*/ token.h[i].link.reset(); //< ok
             }
            }break;
            case OTHER: {}break;
         }
        }
      }
   enum {TOKEN=0,OTHER} type;

   union {
       struct {
           double score;
           struct {
               std::shared_ptr<B> link;
               double to_find;
               } h [4];
       }token;

       struct {
          //else
       } other;
   }
};

My code:

void f()
{
    vector<A> vec;
    A tmp = A(A::TOKEN);
    vec.emplace_back(tmp);
}

Option 1: this causes an error when leaving f; option 2: Ok but ~shared_ptr() is not call, so it make memory leak, right?

If you have an idea that could help me understand who is wrong.

Edit: I use C++11 with gcc.4.6.3 on Ubuntu 12.04x86.

Original code:

    class stack_token {
        public:
            stack_token();
            stack_token(const stack_token& other);
            stack_token(const int i,Parser::peptide::peak* data); //peak
            stack_token(const int i,const double e,AnalyseurPeptide::stack_token* peak); //aa
            stack_token(const int i); //aa pour boucher un trou
            stack_token(const double score); //HEADER

            ~stack_token();

            stack_token& operator=(const stack_token& other);

            inline stack_token* get_peak_stack_NULL() {
                stack_token* res = aa_token.pt_data;
                aa_token.pt_data=NULL;
                return res;
            };

            void __print__() const;


            enum Type {UNKNOW=-1,AA_TOKEN=0,AA_HOLD_TOKEN,/*AA_LIST,*/PEAK_TOKEN, HEADER_TOKEN} type;

            union {
                struct  {
                    int index;
                    double error;
                    stack_token* pt_data;
                } aa_token;

                struct{
                    double error;
                    stack_token* pt_data;
                    std::vector<int> aa_index;
                } aa_hold_token;

                struct {
                    int index;
                    Parser::peptide::peak* pt_data;
                } peak_token;

                struct {
                    double score;
                    struct {
                        std::shared_ptr<std::list<list_arg> > link;
                        double to_find;
                    } holds [Parser::peptide::SIZE];
                } header_token;
            };
    };

 stack_token::~stack_token()
{
switch(type)
{
    case AA_TOKEN:
    {
       if(aa_token.pt_data != NULL)
            delete aa_token.pt_data;
    }break;

    case AA_HOLD_TOKEN :
    {
        aa_hold_token.aa_index.~vector<int>();
    }break;

    case PEAK_TOKEN : 
    {
    }break;

    case HEADER_TOKEN : 
    {
       for (int i=0;i<Parser::peptide::SIZE;++i)
            header_token.holds[i].link.reset();//~shared_ptr<std::list<list_arg> >();
    }break;

    default : break;
}
};


  stack_token::stack_token()
{
this->type = UNKNOW;
};

stack_token::stack_token(const int i,Parser::peptide::peak* data) //peak
{
this->type=PEAK_TOKEN;
peak_token.index = i;
peak_token.pt_data = data;
};

stack_token::stack_token(const int i,const double e,AnalyseurPeptide::stack_token* peak) //aa
{
this->type=AA_TOKEN;
aa_token.error =e;
aa_token.index = i;
aa_token.pt_data = peak;
};

stack_token::stack_token(const int i)
{
this->type=AA_HOLD_TOKEN;
aa_hold_token.error = 0;
aa_hold_token.pt_data = this;
new(&aa_hold_token.aa_index) vector<int>();
};


stack_token::stack_token(const double score) //HEADER
{
this->type = HEADER_TOKEN;
header_token.score = score;
for (int i=0;i<Parser::peptide::SIZE;++i)
    new (&header_token.holds[i].link) shared_ptr<list<list_arg> >;
#warning "add to_find init"
};

Code that fail:

void save_stack(const std::list<stack_token*>& search, std::list<std::vector<stack_token> >& res)
{
    vector<AnalyseurPeptide::stack_token> l;
    auto i=search.begin();
    auto end = search.end();

    stack_token tmp = stack_token(0.f); /* if I remove this */
    l.emplace_back(tmp); /* and this, all is ok */

    while(i!=end)
   {
     l.emplace_back(**i); //< fail here
      ++i;
   }
   res.emplace_back(l);
}

Answer 1

If you're compiling with C++03, the code is illegal, because C++03 doesn't allow types with non-trivial default constructors, copy constructors, assignment operators or destructors in a union. With C++11, the code is illegal, because if the union contains any of the above, the compiler deletes the corresponding member of the union. So your union has no default constructor, copy constructor, assignment or destructor. Which means you can't instantiate it, or use it in any way. And which means that the default constructor needed by A::A(int) doesn't exist, and that the compile should complain when you define this function (or any constructor of A).

If the compiler compiles such code, it means that the compiler doesn't implement the new union stuff correctly, and thus, that you cannot use it.

With regards to what actually happens: I suspect that the compiler is using bitwise copy in the copy constructor of A (rather than refusing to generate it). vec.emplace_back(tmp) uses the copy constructor to create the new element in vec. Bitwise copy means that you end up with two instances of a shared_ptr which point to the same object, but which both have a count of 1. The first one destructs correctly, and the second accesses deleted memory. Boom.

The simplest way to solve your problem is to use boost::variant (which means defining the struct in the union somewhere outside of the union, and giving them a name). If for some reason you cannot use Boost, it's relatively trivial to implement by hand, along the lines of what you are doing. In the union itself, you just have unsigned char token[ sizeof(TokenType) ]; etc., for each non-POD member, with some additional members if necessary to ensure alignment (on most processors, a double will do the trick). You then use reinterpret_cast on the name of the array to get a pointer to the desired type, placement new to initialize it, and explicit destruction to destruct it, much along the lines you've done. And you implement a copy constructor and an assignment operator that work, and take into account the types as well.

(It's not that difficult. I've done it one or two times: for tokens in a parser, for modeling tables which we get from Excel, etc.)

Answer 2

Technical problems:

• union (don't),
• uninitialized,
• rule of three (not taking properly charge of copying)

Design problems:

• Representing types as numbers. Represent types as types.

Keep the knowledge you gained from writing that code, and start from scratch again.

Very little more can be meaningfully said until you post the real code (e.g. swithc will never compile: what you posted is not the real code).