destructor dependency while return from a function

[tom]

I have the section of code listed below, but when I return from main
an exception is thrown from the library, because while returning,
destructor is called on each of the object in the sequence below:
step1: destroy m2
step2: destroy f
step3: destroy m1
notice, "m1" and "m2" have the class type of Message, and will both
use object "f" in the destructor. The problem happens when run to
step3, it uses an already destroyed object f. (dangling pointer).

My question is: Is there any known solution or design pattern to solve
this problem?

#include <iostream>
#include <vector>
#include <hash_map>
#include <cctype>
#include <cassert>
#include <fstream>
#include <sstream>
#include <list>
#include <deque>
#include <algorithm>
#include <numeric>
#include <stack>
#include <queue>
#include <map>
#include <set>


class Message;

class Folder
{
public:
void add_Msg(Message*);
void remove_Msg(Message*);
~Folder()
{
std::cout<<"~Folder()"<<std::endl;
}
private:
std::set<Message*> messages;
};

void Folder::add_Msg(Message* m)
{
messages.insert(m);
}

void Folder::remove_Msg(Message* m)
{
messages.erase(m);
}

class Message
{
public:
Message(const std::string &str = ""):contents(str){}
Message(const Message&);
Message& operator=(const Message&);
~Message();
void save(Folder&);
void remove(Folder&);
private:
std::string contents;
std::set<Folder*> folders;
void put_Msg_in_Folders(const std::set<Folder*>&);
void remove_Msg_from_Folders();
};

Message::Message(const Message& m):contents(m.contents),
folders(m.folders)
{
put_Msg_in_Folders(m.folders);
}

Message& Message:perator=(const Message& m)
{
remove_Msg_from_Folders();
put_Msg_in_Folders(m.folders);
contents = m.contents;
folders = m.folders;

return *this;
}

Message::~Message()
{
remove_Msg_from_Folders();
}

void Message::save(Folder& f)
{
folders.insert(&f);
f.add_Msg(this);
}

void Message::remove(Folder& f)
{
folders.erase(&f);
f.remove_Msg(this);
}

void Message:ut_Msg_in_Folders(const std::set<Folder*>& f)
{
std::set<Folder*>::const_iterator iter = f.begin();
while(iter!=f.end())
{
(*iter)->add_Msg(this);
++iter;
}
}

void Message::remove_Msg_from_Folders()
{
std::set<Folder*>::iterator iter = folders.begin();
std::cout<<reinterpret_cast<int>(*iter)<<std::endl;
while(iter!=folders.end())
{
(*iter)->remove_Msg(this);
++iter;
}
}

int main(int argc, char *argv[])
{
Message m1("s");
Folder f;
m1.save(f);
Message m2(m1);
m2 = m1;
return 0;
}

 

[Alf P. Steinbach]

* tom:

I have the section of code listed below, but when I return from main
an exception is thrown from the library, because while returning,
destructor is called on each of the object in the sequence below:
step1: destroy m2
step2: destroy f
step3: destroy m1
notice, "m1" and "m2" have the class type of Message, and will both
use object "f" in the destructor. The problem happens when run to
step3, it uses an already destroyed object f. (dangling pointer).

My question is: Is there any known solution or design pattern to solve
this problem?

Not really. It's just a question of not keeping pointers to destroyed
objects. Effectively you're doing

struct A{ ~A(){ std::cout << "Bah" << std::endl; } };
struct B{ A* p; void set( A* q ){ p = q; } ~B(){ delete p; } };

int main()
{
B b;
A a;
b.set( &a );
}

Since A-objects are potentially shared between instances of B you should
probably store the pointers as boost::shared_ptr, and make the A
destructor generally inaccessible so that A intstances won't be created
except via dynamic allocation.

#include <iostream>
#include <vector>
#include <hash_map>
#include <cctype>
#include <cassert>
#include <fstream>
#include <sstream>
#include <list>
#include <deque>
#include <algorithm>
#include <numeric>
#include <stack>
#include <queue>
#include <map>
#include <set>


class Message;

class Folder
{
public:
void add_Msg(Message*);
void remove_Msg(Message*);
~Folder()
{
std::cout<<"~Folder()"<<std::endl;
}
private:
std::set<Message*> messages;
};

void Folder::add_Msg(Message* m)
{
messages.insert(m);
}

void Folder::remove_Msg(Message* m)
{
messages.erase(m);
}

class Message
{
public:
Message(const std::string &str = ""):contents(str){}
Message(const Message&);
Message& operator=(const Message&);
~Message();
void save(Folder&);
void remove(Folder&);
private:
std::string contents;
std::set<Folder*> folders;
void put_Msg_in_Folders(const std::set<Folder*>&);
void remove_Msg_from_Folders();
};

Message::Message(const Message& m):contents(m.contents),
folders(m.folders)
{
put_Msg_in_Folders(m.folders);

Here you're effectively modifying the logically const message m, by
modifying the folders it refers to.

Don't do that.

}

Message& Message:perator=(const Message& m)
{
remove_Msg_from_Folders();
put_Msg_in_Folders(m.folders);

Here you're effectively modifying the logically const message m, by
modifying the folders it refers to.

Don't do that.

Anyway, try to express assignment in terms of copy construction.

contents = m.contents;
folders = m.folders;

return *this;
}

Message::~Message()
{
remove_Msg_from_Folders();
}

void Message::save(Folder& f)
{
folders.insert(&f);
f.add_Msg(this);
}

void Message::remove(Folder& f)
{
folders.erase(&f);
f.remove_Msg(this);
}

void Message:ut_Msg_in_Folders(const std::set<Folder*>& f)
{
std::set<Folder*>::const_iterator iter = f.begin();
while(iter!=f.end())
{
(*iter)->add_Msg(this);
++iter;
}
}

void Message::remove_Msg_from_Folders()
{
std::set<Folder*>::iterator iter = folders.begin();
std::cout<<reinterpret_cast<int>(*iter)<<std::endl;
while(iter!=folders.end())
{
(*iter)->remove_Msg(this);
++iter;
}
}

int main(int argc, char *argv[])
{
Message m1("s");
Folder f;
m1.save(f);
Message m2(m1);
m2 = m1;
return 0;
}

 

[tom]

* tom:

I have the section of code listed below, but when I return from main
an exception is thrown from the library, because while returning,
destructor is called on each of the object in the sequence below:
step1: destroy m2
step2: destroy f
step3: destroy m1
notice, "m1" and "m2" have the class type of Message, and will both
use object "f" in the destructor. The problem happens when run to
step3, it uses an already destroyed object f. (dangling pointer).

My question is: Is there any known solution or design pattern to solve
this problem?

Not really. It's just a question of not keeping pointers to destroyed
objects. Effectively you're doing

struct A{ ~A(){ std::cout << "Bah" << std::endl; } };
struct B{ A* p; void set( A* q ){ p = q; } ~B(){ delete p; } };

int main()
{
B b;
A a;
b.set( &a );
}

Since A-objects are potentially shared between instances of B you should
probably store the pointers as boost::shared_ptr, and make the A
destructor generally inaccessible so that A intstances won't be created
except via dynamic allocation.

#include <iostream>
#include <vector>
#include <hash_map>
#include <cctype>
#include <cassert>
#include <fstream>
#include <sstream>
#include <list>
#include <deque>
#include <algorithm>
#include <numeric>
#include <stack>
#include <queue>
#include <map>
#include <set>

class Message;

class Folder
{
public:
void add_Msg(Message*);
void remove_Msg(Message*);
~Folder()
{
std::cout<<"~Folder()"<<std::endl;
}
private:
std::set<Message*> messages;
};

void Folder::add_Msg(Message* m)
{
messages.insert(m);
}

void Folder::remove_Msg(Message* m)
{
messages.erase(m);
}

class Message
{
public:
Message(const std::string &str = ""):contents(str){}
Message(const Message&);
Message& operator=(const Message&);
~Message();
void save(Folder&);
void remove(Folder&);
private:
std::string contents;
std::set<Folder*> folders;
void put_Msg_in_Folders(const std::set<Folder*>&);
void remove_Msg_from_Folders();
};

Message::Message(const Message& m):contents(m.contents),
folders(m.folders)
{
put_Msg_in_Folders(m.folders);

Here you're effectively modifying the logically const message m, by
modifying the folders it refers to.

Don't do that.

}

Message& Message:perator=(const Message& m)
{
remove_Msg_from_Folders();
put_Msg_in_Folders(m.folders);

Here you're effectively modifying the logically const message m, by
modifying the folders it refers to.

Don't do that.

Anyway, try to express assignment in terms of copy construction.

contents = m.contents;
folders = m.folders;

return *this;
}

void Message::save(Folder& f)
{
folders.insert(&f);
f.add_Msg(this);
}

void Message::remove(Folder& f)
{
folders.erase(&f);
f.remove_Msg(this);
}

void Message:ut_Msg_in_Folders(const std::set<Folder*>& f)
{
std::set<Folder*>::const_iterator iter = f.begin();
while(iter!=f.end())
{
(*iter)->add_Msg(this);
++iter;
}
}

void Message::remove_Msg_from_Folders()
{
std::set<Folder*>::iterator iter = folders.begin();
std::cout<<reinterpret_cast<int>(*iter)<<std::endl;
while(iter!=folders.end())
{
(*iter)->remove_Msg(this);
++iter;
}
}

int main(int argc, char *argv[])
{
Message m1("s");
Folder f;
m1.save(f);
Message m2(m1);
m2 = m1;
return 0;
}

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Thanks a lot for your answer. Your advice rocks.
To deal with problem, I think I could only fall back to dynamic memory
allocation and reference counter or smart pointer.(Though it's not a
good idea to put it in a container here in this scenario)

 

[tragomaskhalos]

Thanks a lot for your answer. Your advice rocks.
To deal with problem, I think I could only fall back to dynamic memory
allocation and reference counter or smart pointer.(Though it's not a
good idea to put it in a container here in this scenario)- Hide quoted text -

To put it another way, when programming with containers it's always a
good idea to have a clear picture of who *owns* the contained data
(as opposed to merely referencing it). With this in place, you can
ensure that destruction happens correctly without having to revert to
smart pointers/reference counting. Many folks will tell you that it's
better/safer to use smart pointers anyway, but often just getting
this
ownership issue straight in your head (and in the code!) is
sufficient.