SDL 2.0 Memory Leak

Question

So I have a memeory leak in my SDL2.0 program I managed to get it down to this chunk of code, I don't know if i'm just blind but could anyone see if they can see a memory leak in here?

I thank anyone who responds.

SDL_Color colour = { 255,255,255 };

SDL_Surface* surfaceMessage = nullptr;

SDL_Texture* Message = nullptr;

for (int i = 0; i < m_vItems.size(); i++)
{
    if (surfaceMessage != nullptr) 
    {
        SDL_FreeSurface(surfaceMessage);
    }

    SDL_Color colour = {255,255,255};
    if (i == m_iSelection)
    {
        colour = { 255,0,0 };
    }

    surfaceMessage = TTF_RenderText_Solid(m_Font, m_vItems[i].c_str(), colour); // as TTF_RenderText_Solid could only be used on SDL_Surface then you have to create the surface first

    Message = SDL_CreateTextureFromSurface(renderer, surfaceMessage); //now you can convert it into a texture


    //Get split change
    float thing = screenHeight - 2 * (screenHeight* 0.3);
    thing /= m_vItems.size();

    SDL_Rect Message_rect; //create a rect
    Message_rect.x =  m_iPosX;  //controls the rect's x coordinate 
    Message_rect.y = m_iPosY + (i * thing); // controls the rect's y coordinte
    Message_rect.w = 0.18* screenWidth; // controls the width of the rect
    Message_rect.h = 0.08 * screenHeight; // controls the height of the rect

    SDL_RenderCopy(renderer, Message, NULL, &Message_rect); //you put the renderer's name first, the Message, the crop size(you can ignore this if you don't want to dabble with cropping), and the rect which is the size and coordinate of your texture
}

if (surfaceMessage != nullptr)
{
    SDL_FreeSurface(surfaceMessage);
}

@NathanOliver textures do indeed need their memory freed, though the function is *technically* named [`SDL_DestroyTexture`](https://wiki.libsdl.org/SDL_DestroyTexture). OP might just be best off using `std::unique_ptr` though — jaggedSpire, Aug 02 '16 at 18:37
@jaggedSpire Cool. I never used SDL so it was just a guess as it was a pointer and the surfaced was being cleaned up. — NathanOliver, Aug 02 '16 at 18:38

jaggedSpire · Answer 1 · 2016-08-04T15:21:30.300

Textures need to be destroyed with the SDL_DestroyTexture function, and you're not doing that, which is leading to the memory leak.

Incidentally, manually calling the destruction function on a raw pointer when you no longer need it is error prone (you can easily forget a case where it needs to go) and unnecessary, with the wonderful world of Resource Acquisition Is Initialization (RAII).

Ensuring proper disposal of acquired resources is actually a fairly common requirement in C++, and one it's well suited for. You can get object lifetime to manage resources for you using RAII, which has you place destruction/deallocation code in the destructor of the class, so it's automatically executed when the object managing the resource goes out of scope. This makes exception safety easy as well, since the destructor code is executed even in case of an exception.

In your case, you'd have a class store a pointer to the created SDL object, and then call the relevant SDL end-of-lifetime function for the variety of object you're managing in the destructor, so a texture management class might look like:

class Texture{
SDL_Texture* texture_;
public:
    // or simply pass the arguments for texture creation in and create it internally
    Texture(SDL_Texture* texture) : texture_{texture}{}
    ~Texture(){
        if (texture_ != nullptr){ 
            SDL_DestroyTexture(texture_);
        }
    }

    // ... functions to access texture
};

Notice, though, that you're essentially wrapping a pointer to a resource with atypical end-of-lifetime requirements, and that those requirements are essentially to call a free function that takes the pointer as an argument, provided it's not a null pointer.

You don't need to make a specific class to do that--unique_ptr was designed for this job and will do it admirably if you give it a custom deleter. Giving a unique_ptr a custom deleter is an easy task, too.¹

If you have a function that takes a pointer type T* as its only argument and ends the lifetime of the pointed object and associated memory, it's as simple as this:

#include <memory>

void shoutFooDestruction(Foo* foo); // my special lifetime ending function

int main(){
    std::unique_ptr<Foo, decltype(&shoutFooDestruction)> ptr{
        new Foo(), 
        shoutFooDestruction};    
    ptr.reset(new Foo());
}

This will call shoutFooDestruction twice: once for the original pointer given to the unique pointer at the start of its lifetime, and called when the unique_ptr was reset, and once for the Foo supplied on the reset, when the unique_pointer's lifetime ends at the end of main.²

See it live on Coliru

The equivalent for an SDL_Texture would be:

std::unique_ptr<SDL_Texture, &SDL_DestroyTexture> message{
    nullptr, 
    SDL_DestroyTexture};

and for an SDL_Surface:

std::unique_ptr<SDL_Surface, &SDL_FreeSurface> surfaceMessage{
    nullptr,
    SDL_FreeSurface};

You would then call reset on the unique pointers when you wanted a new texture or surface from SDL, and they would dispose of any old texture or surface for you. Any remaining contents of the unique_ptrs at the end of the function would be dealt with using the same functions when the unique_ptrs went out of scope.

^{1. As seen in constructors 3 and 4 on cppreference's page on constructors for unique pointers}

^{2. Calling reset on a unique pointer with nullptr for its value will not call the provided deleter and the same goes for unique_ptr's destructor, so you don't need to worry about avoiding the deletion call if you don't have anything to delete.}

I've made a more complete write-up on using RAII on C-style interfaces here: [Using RAII to manage resources from a C-style API](http://stackoverflow.com/questions/39176805/using-raii-to-manage-resources-from-a-c-style-api) — jaggedSpire, Aug 30 '16 at 16:42

SDL 2.0 Memory Leak

1 Answers1