EXC_BAD_ACCESS when I'm trying to check for NULL

Question

I'm trying to implement a simple Color Keying engine example based on LazyFoo's awesome tutorials (http://lazyfoo.net/tutorials/SDL/10_color_keying/index.php) but when I try to run it, the thread gives me an EXC_BAD_ACCESS(code=1, address=0x0) on a pointer despite the fact that I'm trying to test whether that pointer is null or not. Here's what the class looks like:

//Texture wrapper class
class LTexture
{
public:
    //Initializes variables
    LTexture();
    //Deallocates memory
    ~LTexture();
    //Loads image at specified path
    bool loadFromFile(std::string path);
    //Deallocates texture
    void free();
    //Renders a texture at a given point
    void render(int x, int y);
    //Gets an image's dimensions
    int getWidth();
    int getHeight();
private:
    //The actual hardware texture
    SDL_Texture* mTexture = NULL;
    //Image dimensions
    int mWidth;
    int mHeight;
};

and here's the intialize and destroy methods:

LTexture::LTexture()
{
    //Initialize
    mTexture = NULL;
    mWidth = 0;
    mHeight = 0;
    printf("I initialized");
}

LTexture::~LTexture()
{
    free();
}

And my error lies in the LTexture::free method.

void LTexture::free()
{
    //Free texture if it exists
    if (mTexture != NULL) //HERE IS THE ISSUE. WHAT IS WRONG WITH THIS?
    {
        SDL_DestroyTexture(mTexture);
        mTexture = NULL;
        mWidth = 0;
        mHeight = 0;
    }
}

As you can see in-line, the issue appears when I test if mTexture is NULL, which I believe should be valid, but for some reason it's not. What am I doing wrong? Will posting more of the code help?

Answer 1

The issue is probably that you do not handle copying and moving correctly.

Whenever you copy the LTexture, only the pointer gets copied. If the copy then goes out of scope, the destructor is called. Then, the original goes out of scope at some point and the destructor is called again on the same pointer leading to double freeing.

I would recommend using a smart pointer:

#include <memory>
class TextureDeleter { void operator()(SDL_Texture* t) { SDL_DestroyTexture(t); };

// in the class
std::unique_ptr<SDL_Texture, TextureDeleter> mTexture;

You can then delete your destructor.

Edit: If you really don't want to use <memory>, then you can achieve the same kind of safety by just adding

LTexture(const LTexture &) = delete;
LTexture& operator=(const LTexture &) = delete;
LTexture(LTexture &&) = delete;
LTexture& operator=(LTexture &&) = delete;

to your class.

But, as pointed out in the comments, this only works if you don't actually need to move or copy your class. If you do, you have to use shared_ptr which is nontrivial to create yourself.