Hello GDNet. I am studying the SDL library and while doing so i find myself in troubles back in the C++ basics.
The question is : If you delete an object that is dynamically allocated, and then try it again does it cause problems or leaks ?

The issue is as follows:

I have a Sprite class which basically deals with loading sprites and drawing them onto the screen.

I have Game class with functions like Init() Draw() Update() Quit().

I dynamically create new sprites in the Game class for each image i want to draw, and then i delete objects in the Quit() function.

In the main game loop i experimented with some things. I attached a key event to delete specific sprite object which actually removes the

sprite from the screen and clears the allocated memory. Note that i did that in the game loop. And in the Quit function i also delete

that object. Is this ok ? It does not seem to crash the program or anything but what happens behind the scenes ?

This can be an issue when the Player kills an Enemy and i free that specific object, and then back in the Quit function i try to
free the garbage again.

Here is some code for reference

game.cpp

 Sprite Background = new Sprite();

Game::Initialize()
{
    Background->LoadSprite("bg.bmp");
    /........../
}

Game::Loop()
{
    while(Running){
      while(PollEvent){
         if(KEY == "x"){
            delete Background;
           Background = NULL;
        }
     }
   }
}

Game::Quit()
{
    //Background once deleted and now again ?
    delete Background;
}

If you try to call delete on a NULL pointer, it will be ignored.

There's no necessity of a null check like:

if (pointer)
    delete pointer;

Even less:

delete pointer; //Free allocated memory.
pointer = NULL;
delete pointer; //Just to make sure.

That second delete in your code is doing nothing, but it detracts from your code's readability, potentially making it confuse to anyone reading it (even you).

Even if it does no harm directly, it is doing no good either, and I would advise against this.

Always prefer to make clear code and it will make your life way easier when a nasty bug appear!

I am confused here. Basically i delete all objects in the Quit function. But if an enemy object gets killed in the main loop where game logic happens i free the texture and delete the object. Also you never know if the enemy will be killed in the main loop so you need to take care of it in the end function also.

So the best practice is to assign NULL to pointers that don't get to play anymore ( if they ) and delete them in the end just to make sure or if they survived game logic ?

delete pointer; //Free allocated memory.
delete pointer; //Just to make sure.
That second delete in your code is doing nothing, but it detracts from your code's readability, potentially making it confuse to anyone reading it (even you).
Even if it does no harm directly, it is doing no good either, and I would advise against this.

Don't do that.

Double-delete is a very serious bug.

The general idiom is that if you are doing your own memory management with new/delete (which is not recommended) to always follow a delete with nullification:

delete foo;

foo = NULL;

That doesn't completely address it, since there may be other pointers that all become invalid after the object is deleted, but it does help with the most common case. Attempting to delete a pointer to NULL does nothing untoward, since the delete operator checks for a null pointer as part of the process.

So the best practice is to assign NULL to pointers that don't get to play anymore ( if they ) and delete them in the end just to make sure or if they survived game logic ?

Nope, C++ doesn't have an automatic garbage collector that will handle that case cleanly.

If you allocate it you must release it. Blindly assigning NULL to pointers means the resources will be leaked unless you clean them up elsewhere. As your program leaks memory and other resources performance will degrade and eventually cause the program to crash or otherwise seriously misbehave.

To clarify dejaime's post, you cannot double delete an object:

int* a = new int;
delete a;
delete a; // <-- NO

... but you can delete a null pointer:

int* a = new int;
delete a;
a = nullptr;
delete a; // <-- OK

So the best practice is to assign NULL to pointers that don't get to play anymore ( if they ) and delete them in the end just to make sure or if they survived game logic ?

Well, no. You cannot assign a pointer to NULL and later expect to delete the object it once referenced. But it is safe to assign a pointer to NULL after deleting it, if you expect to delete it later.

There are some other things you should also consider: Texture resources are usually shared between sprite objects and are maintained outside the sprite's control. This way, you can load from disk once, and reference them freely until the game ends.
Second: You usually use an dynamic array (std::vector) to keep track of game objects. The logic updating those objects typically receives some sort of signal from the object to delete it. From there the logic removes that object from the array, removes the object from the array, and finally deletes the object.

You might also want to consider reading up on object lifetimes and smart pointers.

Don't do that.

Double-delete is a very serious bug.

Sorry about that, fixed the code with what I originally meant to write.

I'm just too used to my overloaded delete that nullifies it for me; my bad.

@ Back on topic,

Under normal circumstances, you delete any objects when you know for sure they won't be reused.
Sometimes that happens during the gameplay, and sometimes at the end.

Still, you can avoid going around calling delete, by, for example, calling a function to make unused objects delete themselves.

#include <algorithms>
#include <vector>

bool exit;

class AwesomeGameObj {
    private:
        static std::vector<AwesomeGameObj*> MasterVector;
        bool isUseless;
    public:
        AwesomeGameObj () {
            /*Initialize everything*/
            isUseless = false;
            MasterVector.pushback(this);
        }
        
        ~AwesomeGameObj () {
            /*Do the cleanup*/
            MasterVector.erase(/*this object*/);
        }
        
        void de_init () {
            while ( !MasterVector.empty() ) {
                delete MasterVector.back();
                MasterVector.pop_back();
            }
        }
        
        bool useless () const {return isUseless;}
        
        static void clean_up () {
            for (/*every object in MasterVector*/)
                if (object->useless()) {
                    vec.erase(/*object*/);
                    delete /*object*/;
                }
        }
        
        static void clean_up (std::vector<AwesomeGameObj*> vec*) {
            for (/*every object in vec*/)
                if ( object->useless() ) {
                    vec->erase(/*object*/);
                    delete /*object*/;
                }
        }
        
        void update () {
            /*do anything it needs to do*/
            if (/*something very very sad happened*/)
                isUseless = true;
        }

};

void quit () {
    AwesomeGameObj::de_init();
    /*do it for any other object types that needs this clean up here*/
}

void init_all () {
    /*init anything*/
}

int main()
{
    //Game Objects in general
    std::vector<AwesomeGameObj*> gameObjects; 
    
    //Fix don't-delete-while-running game objects.
    std::vector<AwesomeGameObj*> miscUseObjects;
    
    init_all();
    
    //Initialize some special objects
    while (/*needs misc objects*/)
        miscUseObjects.push_back (new AwesomeGameObj);
    
    //Main loop
    do {
        while (/*needs new objects*/) {
            gameObjects.push_back (new AwesomeGameObj);
        }
        
        for (/*all gameObjects and miscUseObjects*/) {
            object.update();
        }
        
        if (/*you want to free memory*/)
            AwesomeGameObj::clean_up(gameObjects);
        
    } while (!exit);
    
    //We'll most probably have objects remaining;
    quit();
    
    return 0;
}

As you can see in this example pseudo code, I avoid to call delete on the main and quit functions. I do this by passing on the responsibility of doing so to the object's own class, and all I have to do is tell it to clean up its memory, despite how many objects I need to free and where they are, where they were allocated or anything. This way my main and end functions doesn't have to worry about remaining objects nor freed objects.

Second: You usually use an dynamic array (std::vector) to keep track of game objects.

Do you mean for specific types of objects. How would you store all of game objects in a dynamic array when they all are of different types.
You can of course std::vector<Sprite> spriteList. Also can you check if specific object exists in the list before deletion ?

Anyway this :

int* a = new int;
delete a;
a = nullptr;
delete a; // <-- OK

means after delete pointer assign nullptr and then delete the null pointer which is safe and so it solves my problem! Thanks !

Why don't you use std::unique_ptr's anyway?

Assuming a recent compiler, follow ProtectedMode's advice:

std::unique_ptr<Sprite> Background;

Game::Initialize()
{
    // may require a _very_ recent compiler, as make_unique is technically part of the
    // yet-to-be-released C++14, but Visual Studio 2013 has it
    Background = std::make_unique<Sprite>();
    // alternative if you don't have make_unique:
    // Background.reset(new Sprite());
    Background->LoadSprite("bg.bmp");
    /........../
}

Game::Loop()
{
    while(Running){
      while(PollEvent){
         if(KEY == "x"){
           // automatically deletes Background
           Background = nullptr;
        }
     }
   }
}

Game::Quit()
{
    // automatically deletes Background
    // totally safe to call again and again
    Background = nullptr;
}

std::unique_ptr will ensure that the object it owns is deleted if you assign something else to it (including nullptr/NULL). Note that you can only have a single std::unique_ptr to a particular object at a time. Use plain pointers or references (Sprite&) if you need to borrow the object in a function signature or the like. I would avoid std::shared_ptr; it's an enticing trap for the unwary.

more on unique_ptr here

I may have missed something, but is there a reason to keep the Background object as a pointer? Would it be better to keep it on the stack and rely on it being automatically cleaned up when the Game object dies?

You obviously use a load function to get the texture into memory, you could also use an unload function to remove it from memory if the need exists. And deciding if it should be drawn or not is also something that should exist if you have such requirements, it's easier to give it a bool isDrawn, and instead of deleting the object, simply set it as isDrawn = false. Seems like a nicer choice to me.