deleting a composite, double-delete problem

Discussion in 'C++' started by Matan Nassau, May 24, 2004.

  1. Matan Nassau

    Matan Nassau Guest

    Hello.

    i have a composite which i want to delete. this is a composite which
    represents a boolean expression (see a previous post of mine with more
    details at http://groups.google.ca/groups?hl=e...atan+nassau&hl=en&lr=&ie=UTF-8&sa=G&scoring=d
    )

    VariableExp *x = new VariableExp("X");
    VariableExp *y = new VariableExp("Y");
    BooleanExp expression(
    new OrExp(
    new AndExp(new Constant(true), x),
    new AndExp(x, y) ) );

    represents the expression
    (TRUE AND X) OR (X AND Y)

    i have defined destructors for these dervied expressions as follows:

    AndExp::~AndExp()
    {
    if (_operand1) {
    delete _operand1;
    _operand1 = 0;
    }
    if (_operand2) {
    delete _operand2;
    _operand2 = 0;
    }
    }

    OrExp::~OrExp()
    {
    if (_operand1) {
    delete _operand1;
    _operand1 = 0;
    }
    if (_operand2) {
    delete _operand2;
    _operand2 = 0;
    }
    }
    NotExp::~NotExp()
    {
    if (_operand) {
    delete _operand;
    _operand = 0;
    }
    }

    for deleting the expression this way
    delete expression;

    i know this doesn't do the job, as the second AndExp in my example
    here doesn't know that the first AndExp already deleted the
    VariableExp x, and it tries to delete it - causing an error for
    double-deleting.
    my question is, how do I avoid it? what technique should i use to
    delete the composite properly?

    TIA,

    Matan Nassau
     
    Matan Nassau, May 24, 2004
    #1
    1. Advertising

  2. Matan Nassau wrote:
    > i have a composite which i want to delete. this is a composite which
    > represents a boolean expression (see a previous post of mine with more
    > details at http://groups.google.ca/groups?hl=e...atan+nassau&hl=en&lr=&ie=UTF-8&sa=G&scoring=d
    > )
    >
    > VariableExp *x = new VariableExp("X");
    > VariableExp *y = new VariableExp("Y");
    > BooleanExp expression(
    > new OrExp(
    > new AndExp(new Constant(true), x),
    > new AndExp(x, y) ) );
    >
    > represents the expression
    > (TRUE AND X) OR (X AND Y)
    >
    > i have defined destructors for these dervied expressions as follows:
    >
    > AndExp::~AndExp()
    > {
    > if (_operand1) {
    > delete _operand1;
    > _operand1 = 0;
    > }
    > if (_operand2) {
    > delete _operand2;
    > _operand2 = 0;
    > }


    There is no need for those tests. 'delete 0' is a NOP. Just write

    delete _operand1;
    delete _operand2;

    And setting them to 0 is also not needed -- the object is going away,
    the values of its members are not accessible after the destructor is
    done.

    > }
    >
    > OrExp::~OrExp()
    > {
    > if (_operand1) {
    > delete _operand1;
    > _operand1 = 0;
    > }
    > if (_operand2) {
    > delete _operand2;
    > _operand2 = 0;
    > }


    Same note as above.

    > }
    > NotExp::~NotExp()
    > {
    > if (_operand) {
    > delete _operand;
    > _operand = 0;
    > }


    Same note.

    > }
    >
    > for deleting the expression this way
    > delete expression;
    >
    > i know this doesn't do the job, as the second AndExp in my example
    > here doesn't know that the first AndExp already deleted the
    > VariableExp x, and it tries to delete it - causing an error for
    > double-deleting.
    > my question is, how do I avoid it? what technique should i use to
    > delete the composite properly?


    You've run into the classical problem of a shared pointer. The only
    recommendation I have for shared pointers is: do _not_ use them.

    The main question you need answered is: who _OWNS_ those objects.
    Only the owner should be allowed to dispose of them (if they were
    allocated dynamically). It often is considered OK if the object owns
    itself. In order to respond properly to others' desire to dispose of
    it it, the object has to count its uses (references). Take a look at
    any existing reference counting schemes and use the simplest one. Or
    use the most robust. In any case, it comes to the object deleting
    itself when all references are released (reference counter comes to 0).

    Victor
     
    Victor Bazarov, May 24, 2004
    #2
    1. Advertising

  3. Matan Nassau

    Buster Guest

    Matan Nassau wrote:
    > Hello.
    >
    > i have a composite which i want to delete. this is a composite which
    > represents a boolean expression (see a previous post of mine with more
    > details at http://groups.google.ca/groups?hl=e...atan+nassau&hl=en&lr=&ie=UTF-8&sa=G&scoring=d
    > )
    >
    > VariableExp *x = new VariableExp("X");
    > VariableExp *y = new VariableExp("Y");
    > BooleanExp expression(
    > new OrExp(
    > new AndExp(new Constant(true), x),
    > new AndExp(x, y) ) );
    >
    > represents the expression
    > (TRUE AND X) OR (X AND Y)
    >
    > i have defined destructors for these dervied expressions as follows:
    >
    > AndExp::~AndExp()
    > {
    > if (_operand1) {
    > delete _operand1;
    > _operand1 = 0;
    > }
    > if (_operand2) {
    > delete _operand2;
    > _operand2 = 0;
    > }
    > }
    >
    > OrExp::~OrExp()
    > {
    > if (_operand1) {
    > delete _operand1;
    > _operand1 = 0;
    > }
    > if (_operand2) {
    > delete _operand2;
    > _operand2 = 0;
    > }
    > }
    > NotExp::~NotExp()
    > {
    > if (_operand) {
    > delete _operand;
    > _operand = 0;
    > }
    > }
    >
    > for deleting the expression this way
    > delete expression;
    >
    > i know this doesn't do the job, as the second AndExp in my example
    > here doesn't know that the first AndExp already deleted the
    > VariableExp x, and it tries to delete it - causing an error for
    > double-deleting.


    You have given ownership of the VariableExp object to two different
    objects. (We say an object x 'owns' a dynamically-allocated object
    y if x destroys y in its destructor.)

    Two solutions spring to mind:

    (i) avoid the situation of shared ownership by passing pointers to
    distinct objects to the constructors.

    (ii) deal with shared ownership using boost::shared_pointer. You should
    be able to work out the details. See the Boost website for documentation.

    > my question is, how do I avoid it? what technique should i use to
    > delete the composite properly?


    --
    Regards,
    Buster.
     
    Buster, May 24, 2004
    #3
  4. "Matan Nassau" <> wrote in message
    news:...
    > Hello.
    >
    > i have a composite which i want to delete. this is a composite which
    > represents a boolean expression (see a previous post of mine with more
    > details at

    http://groups.google.ca/groups?hl=e...atan+nassau&hl=en&lr=&ie=UTF-8&sa=G&scoring=d
    > )
    >
    > VariableExp *x = new VariableExp("X");
    > VariableExp *y = new VariableExp("Y");
    > BooleanExp expression(
    > new OrExp(
    > new AndExp(new Constant(true), x),
    > new AndExp(x, y) ) );
    >
    > represents the expression
    > (TRUE AND X) OR (X AND Y)
    >
    > i have defined destructors for these dervied expressions as follows:
    >
    > AndExp::~AndExp()
    > {
    > if (_operand1) {
    > delete _operand1;
    > _operand1 = 0;
    > }
    > if (_operand2) {
    > delete _operand2;
    > _operand2 = 0;
    > }
    > }
    >
    > OrExp::~OrExp()
    > {
    > if (_operand1) {
    > delete _operand1;
    > _operand1 = 0;
    > }
    > if (_operand2) {
    > delete _operand2;
    > _operand2 = 0;
    > }
    > }
    > NotExp::~NotExp()
    > {
    > if (_operand) {
    > delete _operand;
    > _operand = 0;
    > }
    > }
    >
    > for deleting the expression this way
    > delete expression;
    >
    > i know this doesn't do the job, as the second AndExp in my example
    > here doesn't know that the first AndExp already deleted the
    > VariableExp x, and it tries to delete it - causing an error for
    > double-deleting.
    > my question is, how do I avoid it? what technique should i use to
    > delete the composite properly?
    >


    This is a situation tailor made for a reference counted smart pointer. See
    shared_ptr at http://www.boost.org/libs/smart_ptr/smart_ptr.htm for
    instance.

    Each smart pointer hold a reference count of how many other object are
    referring to it, and only deletes itself when that count reaches zero.
    Essentially you hand repsonsibility for the delete over to the smart pointer
    class which only deletes when nothing can access the object any more.

    The only change you are likely to have to make would be to replace raw
    pointers with smart ones and remove the deletes from your code.

    class AndExp
    {
    public:
    ~AndExp()
    {
    delete _operand1;
    delete _operand2;
    }
    private:
    BooleanExp* _operand1;
    BooleanExp* _operand2;
    };

    becomes

    class AndExp
    {
    public:
    // no destructor
    private:
    boost::shared_ptr<BooleanExp> _operand1;
    boost::shared_ptr<BooleanExp> _operand2;
    };

    john
     
    John Harrison, May 24, 2004
    #4
  5. Matan Nassau

    Matan Nassau Guest

    "John Harrison" <> wrote in message news:<>...
    >
    > This is a situation tailor made for a reference counted smart pointer. See
    > shared_ptr at http://www.boost.org/libs/smart_ptr/smart_ptr.htm for
    > instance.


    Actually I already took a look at it before, and hesitated about it,
    because I wasn't sure this is a problem a shared_ptr is designed to
    solve. you cleared my doubts.

    now everything is done, and life is good again.
    ...Well, almost. Because i don't understand how come it works. :)
    you see, in addition to my existing constructor,

    AndExp::AndExp(BooleanExp *op1, BooleanExp *op2)
    {
    _operand1.reset(op1);
    _operand2.reset(op2);
    }

    I have a constructor which accepts a shared_ptr:

    typedef boost::shared_ptr<BooleanExpPtr> BooleanExpPtr;

    AndExp::AndExp(BooleanExpPtr op1, BooleanExpPtr op2)
    {
    _operand1 = op1;
    _operand2 = op2;
    }

    (see previous post for details regarding my definitions).
    and I call it with

    typedef boost::shared_ptr<VariableExp> VariableExpPtr;

    VariableExpPtr x(new VariableExp("X"));
    VariableExpPtr y(new VariableExp("Y"));
    BooleanExpPtr expression(new AndExp(x, y));

    and it works. it compiles nicely with no complaints.
    how come? just let me understand the concept of the shared_ptr:
    VariableExp derives from BooleanExp so VariableExp is a BooleanExp.
    Good. But VariableExpPtr does /not/ derive from BooleanExpPtr as much
    as I understand the shared_ptr; it is merely a different shared_ptr.
    so if it is not a BooleanExpPtr, which is what the constructor of
    AndExp expects, how come i don't get a compiler error?

    Thanks,

    Matan
     
    Matan Nassau, May 26, 2004
    #5
  6. (Matan Nassau) wrote in message news:<>...
    > I have a constructor which accepts a shared_ptr:
    >
    > typedef boost::shared_ptr<BooleanExpPtr> BooleanExpPtr;
    >
    > AndExp::AndExp(BooleanExpPtr op1, BooleanExpPtr op2)
    > {
    > _operand1 = op1;
    > _operand2 = op2;
    > }
    >
    > (see previous post for details regarding my definitions).
    > and I call it with
    >
    > typedef boost::shared_ptr<VariableExp> VariableExpPtr;
    >
    > VariableExpPtr x(new VariableExp("X"));
    > VariableExpPtr y(new VariableExp("Y"));
    > BooleanExpPtr expression(new AndExp(x, y));
    >
    > and it works. it compiles nicely with no complaints.
    > how come? just let me understand the concept of the shared_ptr:
    > VariableExp derives from BooleanExp so VariableExp is a BooleanExp.
    > Good. But VariableExpPtr does /not/ derive from BooleanExpPtr as much
    > as I understand the shared_ptr; it is merely a different shared_ptr.
    > so if it is not a BooleanExpPtr, which is what the constructor of
    > AndExp expects, how come i don't get a compiler error?


    Because boost::shared_ptr is designed to support Derived->Base
    conversions just like classic pointers. There's a templated
    implicit conversion. This is used to create a temporary
    BooleanExpPtr from the VariableExpPtr. This temporary shares
    the reference count with the original VariableExpPtr, so it
    works as expected. The VariableExp isn't destroyed while the
    BooleanExpPtr points to it.

    This is a common advantage of boost libraries. The tricky cases have
    been thought through for you.

    Regards,
    Michiel Salters
     
    Michiel Salters, May 27, 2004
    #6
  7. (Matan Nassau) wrote in message news:<>...
    > I have a constructor which accepts a shared_ptr:
    >
    > typedef boost::shared_ptr<BooleanExpPtr> BooleanExpPtr;
    >
    > AndExp::AndExp(BooleanExpPtr op1, BooleanExpPtr op2)
    > {
    > _operand1 = op1;
    > _operand2 = op2;
    > }
    >
    > (see previous post for details regarding my definitions).
    > and I call it with
    >
    > typedef boost::shared_ptr<VariableExp> VariableExpPtr;
    >
    > VariableExpPtr x(new VariableExp("X"));
    > VariableExpPtr y(new VariableExp("Y"));
    > BooleanExpPtr expression(new AndExp(x, y));
    >
    > and it works. it compiles nicely with no complaints.
    > how come? just let me understand the concept of the shared_ptr:
    > VariableExp derives from BooleanExp so VariableExp is a BooleanExp.
    > Good. But VariableExpPtr does /not/ derive from BooleanExpPtr as much
    > as I understand the shared_ptr; it is merely a different shared_ptr.
    > so if it is not a BooleanExpPtr, which is what the constructor of
    > AndExp expects, how come i don't get a compiler error?


    Because boost::shared_ptr is designed to support Derived->Base
    conversions just like classic pointers. There's a templated
    implicit conversion. This is used to create a temporary
    BooleanExpPtr from the VariableExpPtr. This temporary shares
    the reference count with the original VariableExpPtr, so it
    works as expected. The VariableExp isn't destroyed while the
    BooleanExpPtr points to it.

    This is a common advantage of boost libraries. The tricky cases have
    been thought through for you.

    Regards,
    Michiel Salters
     
    Michiel Salters, May 27, 2004
    #7
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. sleigh
    Replies:
    1
    Views:
    2,739
    sleigh
    Feb 12, 2004
  2. Sydex
    Replies:
    12
    Views:
    6,649
    Victor Bazarov
    Feb 17, 2005
  3. Harry Barker
    Replies:
    2
    Views:
    544
    Alf P. Steinbach
    Apr 19, 2006
  4. Chad
    Replies:
    0
    Views:
    254
  5. Mike

    Composite vs non composite Controls

    Mike, Mar 10, 2005, in forum: ASP .Net Web Controls
    Replies:
    4
    Views:
    292
    Sundararajan
    Mar 11, 2005
Loading...

Share This Page