1

I am writing an Image class for a project using the raw data from stb_image. In the destructor for this class I am freeing the pointer to the image data to avoid memory leaks. However, when the destructor is called and the data is freed, I get an access violation.

The header of image:

class Image {
    unsigned char* pixelData;
    public:
        int nCols, nRows, nChannels;
        Image(const char* filepath);
        Image(unsigned char* data, int nCols, int nRows, int nChannels);
        Image();
        ~Image();
        Image getBlock(int startX, int startY, int blockSize);
        std::vector<unsigned char> getPixel(int x, int y);
        void writeImage(const char* filepath);

};

The constructor and destructor of Image:

#define STB_IMAGE_IMPLEMENTATION
#define STB_IMAGE_WRITE_IMPLEMENTATION

#include "stb_image.h"
#include "stb_image_write.h"
#include "image.hpp"

Image::Image(const char* filepath) {
    pixelData = stbi_load(filepath, &nCols, &nRows, &nChannels, 0);
    assert(nCols > 0 && nRows > 0 && nChannels > 0);
    std::cout << "Image width: " << nCols << "\nImage height: " << nRows << "\nNumber of channels: " << nChannels << "\n";
}

Image::~Image() {
    stbi_image_free(this->pixelData);
}

Minimum reproducable example:

int main() {
    Image image;
    image = Image("./textures/fire.jpg");
}
Ben Hommrich
  • 13
  • 4
  • 2
    Your `Image` class cannot be copied or assigned safely. Please post a [mcve] that shows your usage of this class. For example `int main() { Image im("somefile");; ... Image im2 = im; }` -- That code right there will cause issues. Read up on the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three), and go to the **Managing Resources** section of that link. – PaulMcKenzie Dec 10 '20 at 22:24
  • 1
    Also, put a breakpoint in the destructor. Do not be surprised if it is called multiple times, where each time `pixelData` is the same pointer value. If that happens, then I am inclined to close this question as a duplicate of the rule of 3 link. – PaulMcKenzie Dec 10 '20 at 22:32
  • I tried putting in the breakpoint like you said and it did get called multiple times with the same pointer value, so that is probably it then. – Ben Hommrich Dec 10 '20 at 22:41
  • image = Image("./textures/fire.jpg"); This is copy assignment from a temporary (xvalue). So, it first creates the object and passes the pixelData pointer to the image instance. But, right after this line, pixelData is freed coz temporary object is destructed. So, eventually, there is already freed pointer inside image instance. – panky Dec 10 '20 at 22:51

1 Answers1

0

Default copy constructor would lead to double free. There are two ways to fix it -

  1. You can wrap the resource with unique_ptr having custom deleter and this would make the class move-only anyways.
  2. Or, make it non-copyable explicitly.
panky
  • 50
  • 4