Why is my program crashing when using multiple classes and pointers?

[JonnyG]

Following my book, we are making a program with two classes: Message and Folder. The idea is that messages will be stored in a folder. The same message can be saved in multiple folders. In order to accomplish this, Message has a set of pointers to Folder that point to each of the folders that contains it. Folder has a set of pointers to Message that point to each of the messages it contains. Here is my code:

#ifndef MESSAGE_HPP
#define MESSAGE_HPP
#include <string>
#include <set>
#include "Folder.hpp"

using std::string;
using std::set;

class Message
{
   
friend class Folder;
public:
    explicit Message(const string &str) : contents(str) {}
    Message(const Message&);
    Message& operator=(const Message&);
    ~Message();
   
    void save(Folder&);
    void remove(Folder&);
   
private:
    string contents;
    set<Folder*> folders;
   
    void addToFolders(const Message&);
    void removeFromFolders();
    void swap(Message&, Message&);
};

#endif

#include "Message.hpp"

Message::Message(const Message &msg) : contents(msg.contents), folders(msg.folders) {
    addToFolders(msg);
}

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

Message& Message::operator=(const Message &rhs) {
    removeFromFolders();
    contents = rhs.contents;
    folders = rhs.folders;
    addToFolders(rhs);
    return *this;
}

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

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

void Message::addToFolders(const Message &msg) {
    for (auto f : msg.folders)
        f->addMsg(this);
}

void Message::removeFromFolders() {
    for (auto f : folders)
        f->remMsg(this);
}

void Message::swap(Message &lhs, Message &rhs) {
    using std::swap;
    for (auto f : lhs.folders)
        f->remMsg(&lhs);
   
    for (auto f: rhs.folders)
        f->remMsg(&rhs);
   
    swap(lhs.folders, rhs.folders);
    swap(lhs.contents, rhs.contents);
   
    for (auto f : lhs.folders)
        f->addMsg(&lhs);
       
    for (auto f : rhs.folders)
        f->addMsg(&rhs);
}

#ifndef FOLDER_HPP
#define FOLDER_HPP
#include <set>

using std::set;

class Message;
class Folder
{

public:
    void addMsg(Message*);
    void remMsg(Message*);
   
private:
   
    set<Message*> containedMessages;
};

#endif

#include "Folder.hpp"

void Folder::addMsg(Message *msg) {
    containedMessages.insert(msg);
}

void Folder::remMsg(Message *msg) {
    containedMessages.erase(msg);
}

My test code that I run in my main.cpp file is (I've removed include directives and other statements for brevity):

int main() {
    Message msg("testing");
    Folder inbox;
    msg.save(inbox);

}

This program crashes. Playing with the program, looking at my debugger, and using a memory checker program, it appears the problem is ultimately coming from my remMsg function in the Folder.cpp file. But I can't figure out exactly what's happening. My memory checker gives errors that say unaddressable memory and invalid heap argument, so I know it has something to do with memory getting deleted and dangling pointers, but I can't seem to fix the program.

Any tips?

Edit: I've suspected that when reaching the end of the main function, my Folder object may be getting deleted before my Message object, which leaves the set of pointers that point to my folder dangling (this set is a data member of my Message object). I would need to have access to the Message class members in my Folder destructor to account this possibility, but I cannot #include my Message header in my Folder header, because the Folder header is already #included in my Message header.

 

[Jarvis323]

Following my book, we are making a program with two classes: Message and Folder. The idea is that messages will be stored in a folder. The same message can be saved in multiple folders. In order to accomplish this, Message has a set of pointers to Folder that point to each of the folders that contains it. Folder has a set of pointers to Message that point to each of the messages it contains. Here is my code:

#ifndef MESSAGE_HPP
#define MESSAGE_HPP
#include <string>
#include <set>
#include "Folder.hpp"

using std::string;
using std::set;

class Message
{

friend class Folder;
public:
    explicit Message(const string &str) : contents(str) {}
    Message(const Message&);
    Message& operator=(const Message&);
    ~Message();

    void save(Folder&);
    void remove(Folder&);

private:
    string contents;
    set<Folder*> folders;

    void addToFolders(const Message&);
    void removeFromFolders();
    void swap(Message&, Message&);
};

#endif

#include "Message.hpp"

Message::Message(const Message &msg) : contents(msg.contents), folders(msg.folders) {
    addToFolders(msg);
}

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

Message& Message::operator=(const Message &rhs) {
    removeFromFolders();
    contents = rhs.contents;
    folders = rhs.folders;
    addToFolders(rhs);
    return *this;
}

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

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

void Message::addToFolders(const Message &msg) {
    for (auto f : msg.folders)
        f->addMsg(this);
}

void Message::removeFromFolders() {
    for (auto f : folders)
        f->remMsg(this);
}

void Message::swap(Message &lhs, Message &rhs) {
    using std::swap;
    for (auto f : lhs.folders)
        f->remMsg(&lhs);

    for (auto f: rhs.folders)
        f->remMsg(&rhs);

    swap(lhs.folders, rhs.folders);
    swap(lhs.contents, rhs.contents);

    for (auto f : lhs.folders)
        f->addMsg(&lhs);
   
    for (auto f : rhs.folders)
        f->addMsg(&rhs);
}

#ifndef FOLDER_HPP
#define FOLDER_HPP
#include <set>

using std::set;

class Message;
class Folder
{

public:
    void addMsg(Message*);
    void remMsg(Message*);

private:

    set<Message*> containedMessages;
};

#endif

#include "Folder.hpp"

void Folder::addMsg(Message *msg) {
    containedMessages.insert(msg);
}

void Folder::remMsg(Message *msg) {
    containedMessages.erase(msg);
}

My test code that I run in my main.cpp file is (I've removed include directives and other statements for brevity):

int main() {
    Message msg("testing");
    Folder inbox;
    msg.save(inbox);

}

This program crashes. Playing with the program, looking at my debugger, and using a memory checker program, it appears the problem is ultimately coming from my remMsg function in the Folder.cpp file. But I can't figure out exactly what's happening. My memory checker gives errors that say unaddressable memory and invalid heap argument, so I know it has something to do with memory getting deleted and dangling pointers, but I can't seem to fix the program.

Any tips?

It looks like in removeFromFolders, called from msg's destructor, it dereferences a pointer to inbox (->remMsg). But when the main function exits, inbox is destroyed before msg is, so when msg's destructor is called, the pointer to inbox is an invalid pointer. You can try printing something in their destructors to see what's happening more clearly.

 

[JonnyG]

Ah yes, your suggestion to print statements in the destructors have confirmed my suspicions. Thank you, Jarvis.

If you see my edit in my OP, I suspected this may have been the problem but I couldn't access the Message data members in the Folder class because Folder class already #included the Message class, so I couldn't #include the Message class in the Folder class.

What I did was create one header for both classes and I made Folder a friend of Message. This gave me access to the Message data members and allowed me to write this in the Folder destructor:

Folder::~Folder() {
    for (auto m : containedMessages)
        m->folders.erase(this);
}

This ensures that there will be no dangling pointers if Folder happens to be deleted first.

Now I am starting to understand what is meant by C++ makes it much harder to shoot yourself in the foot than C does, but when you do it blows off your entire leg.

 

[JonnyG]

@Jarvis323

One more question if you don't mind. I notice that I have been changing the size of my set container with range-for loops by erasing and inserting objects. I was under the impression that this was a very bad idea because it could cause a big error. Any idea why my program is working perfectly fine?

 

[Jarvis323]

@Jarvis323

One more question if you don't mind. I notice that I have been changing the size of my set container with range-for loops by erasing and inserting objects. I was under the impression that this was a very bad idea because it could cause a big error. Any idea why my program is working perfectly fine?

It looks like you're not adding or removing any objects to the container you're looping over, but just to the containers held by those objects. That should be fine.

 

[JonnyG]

Makes sense. Thank you!