Deleting a dynamic member through a destructor causes program to hang

Question

This is my main working class that does all the high level operations:

class Image
{
public:
    Image();
    Image(const Image& copy);
    ~Image();

    void loadimage(string filename);
    void saveimage(string filename);
    Image superimpose(const Image& ontop, Color mask);

    int getwidth();
    int getheight();
    Image operator=(const Image&);

protected:    
    vector< vector<Color> > pixels;
    int width;
    int height;
    ImageLoader* loader;
};

It has a copy constructor:

Image::Image(const Image& copy)
{
    width = copy.width;
    height = copy.height;
    loader = copy.loader;

    pixels = copy.pixels;
}

and an overloaded operator= method:

Image Image::operator=(const Image& other)
{   
    width = other.width;
    height = other.height;
    loader = other.loader;

    pixels = other.pixels

    // Return this instance
    return *this;
}

The destructor is:

Image::~Image()
{
    delete loader;
}

The loadimage() method created a new dynamically allocated loader to appear:

if(magic_number == "P3")
{
    loader = new P3Loader;
}
else if (magic_number == "P6")
{
    loader = new P6Loader;
}
else
{
    exit(1);  // If file is of an inappropriate format
}

When I run the program, it hangs.

EDIT: The post has been edited to reflect the problem. Refer to Praetorian's solution that fixed the problem.

`a = b;` means you're copying the contents of `b` to `a`. You want `b = a;` — Praetorian, May 26 '13 at 01:46
@Praetorian It's a typo. Either one doesn't work. Fixed the typo. — B.K., May 26 '13 at 01:53
And what does *doesn't work* mean? Compiler error? Incorrect results after the copy? If it's the latter, then post what the contents of both `a` and `b` are after the copy. You have a `vector>` which is absolutely copyable. — Praetorian, May 26 '13 at 01:55
The program never executes, it just hangs. Then I get RUN FAILED (exit value 1, total time: 1m 3s). I'm trying to implement the copying in my copy constructor and operator= overload method. Everything in the program works like a charm, until I try to use the copy constructor or the overloaded = method on the objects that have these vectors as part of them. — B.K., May 26 '13 at 02:03
You need to post your class definition, and how you're implementing the copy constructor and assignment operator. The `vector>` member is not your problem, or at least the problem is not what you're thinking it is. If I had to guess, since the copy constructor and assignment operator are involved, I'd say the most likely cause is you're not performing a deep copy of some other resource your class is managing. — Praetorian, May 26 '13 at 02:08
For future reference, since your class manages a resource you should understand [The Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). — Blastfurnace, May 26 '13 at 03:35

Praetorian · Accepted Answer · 2013-05-26T02:37:28.123

You haven't posted how the Image::loader member gets initialized, but I'm assuming it is something like this:

Image::Image()
: loader(nullptr)
{
  // do stuff
  // then initialize loader
  loader = new ImageLoader(...);
}

Similarly, the definition of ~Image() is missing, so I'm making another assumption:

Image::~Image()
{
  delete loader;
}

In the copy constructor (and assignment operator) you're simply copy the pointer from the other Image object, so that both loader members now point to the same ImageLoader object. When the destructor of the first object runs it'll delete the ImageLoader object, and the destructor of the second object will then attempt to delete the same object again. This double deletion is undefined behavior, and is probably why your program crashes.

You need to make a deep copy of the ImageLoader object within the copy constructor and assignment operators. It'll probably look like this:

loader = new ImageLoader(*other.loader); // invokes ImageLoader's copy ctor

You're absolutely right. The destructor and copying of the address rather than contents was the problem. — B.K., May 26 '13 at 02:46

Deleting a dynamic member through a destructor causes program to hang

1 Answers1