deleting a composite, double-delete problem

[Matan Nassau]

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=en...%26scoring%3Dd
)

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

[Victor Bazarov]

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=en...%26scoring%3Dd
)

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

[John Harrison]

"Matan Nassau" <Fo************@ICQMail.com> wrote in message
news:3c**************************@posting.google.c om...

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=en...%26scoring%3Dd )

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

[Buster]

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=en...%26scoring%3Dd
)

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.

[Matan Nassau]

"John Harrison" <jo*************@hotmail.com> wrote in message news:<2h************@uni-berlin.de>...

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

[Michiel Salters]

Fo************@ICQMail.com (Matan Nassau) wrote in message news:<3c**************************@posting.google. com>...

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]

Fo************@ICQMail.com (Matan Nassau) wrote in message news:<3c**************************@posting.google. com>...

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