delete[] object causes crash!!!

[hakeris1010]

Hello. I made a dynamic array of objects. When i call delete[] , program crashes and stops responding. But it does strange behavior: look at this code and output.

//main file
#include "myobject.h"

int size;
myObject *foo;

//some operations to estimate size

foo = new myObject[size];

//some operations with myObject

std::cout<<"size: "<<size<<"\n";
std::cout<<"Deleting object\n";

size=0;
delete [] foo;

//myobject.h

class myObject
{
    public:
    int number;

    Object1 ob1[64]
    Object2 *ob2;

    myObject(){ };
    ~myObject()
    {
        std::cout<<"delete "<<number<<"\n";

        delete [] ob1;
        delete [] ob2;
    };
}

And the strange output:

size: 11
Deleting object
delete 10
delete 9
delete 8
delete 7
delete 6
delete 5
delete 4
delete 3
delete 2
delete 1
delete 0

And then it crashes and stops responding.

[cire]

But it does strange behavior:

Your code has what we call "undefined behavior." It could do anything.

Don't delete[] stuff you don't allocate with new[].

[hakeris1010]

But the problem is that i allocated everything.

[coder777]

But the problem is that i allocated everything.

Nope, line 16: ob1 is not allocated with new[]

[hakeris1010]

ob1 and ob2 are allocated during the process i have not shown

[tipaye]

Object1 ob1[64]

assuming you're just not showing the ";" either.

Here you have created an array of 64 Object1 called ob1 on the stack. It cannot be deallocated using "delete []"

I don't see how you can claim to have legally allocated it elsewhere.

In any case, since it's a major point, please show the "process" that allocates it.

[hakeris1010]

there is a

;

symbol after that declaration, i just forgot to write it here. Also, the process where i allocated it is inside a for loop, its long, so i won't write it here. But it goes through all array and assigns a value to every element.
Also, ob1 and ob2 are structs, not classes.

[Disch]

ob1 and ob2 are allocated during the process i have not shown

If you're having problems with allocation/deallocation and you want our help with it, we're going to have to see what you're doing. We can't tell you what's going wrong when you don't show us relevant code.

Also, the process where i allocated it is inside a for loop, its long, so i won't write it here. But it goes through all array and assigns a value to every element.

Every new[] must have a matching delete[]
Every new must have a matching delete.

If you are allocating individual elements with new in a loop, then you must delete those individual elements with delete in a loop.




Either that or just use smart pointers so you don't have to deal with any of this crap.

[tipaye]

it goes through all array and assigns a value to every element

assigning a value is not the same thing as allocating memory using new

ob1 and ob2 are structs, not classes

What's the difference between a struct and a class? Why have you brought it up?

[hakeris1010]

This code is just like the first one, however, it doesn't crash. So even if i delete something that hasn't been allocated with "new", it works.

This code:

#include <iostream>
#include <fstream>
#include <string>

using namespace std;

struct Kuprine
{
    int knyg;
};

class Mokinys
{
public:
    int num;
    Kuprine *kupr;
    Kuprine kup2[4];

    Mokinys()
    {

    }
    ~Mokinys()
    {
        cout<<num<<endl;
        delete [] kupr;
        delete [] kup2;
    }
};

int main()
{
    Mokinys *mok;

    int size=5;

    mok=new Mokinys[size];

    for(int i=0; i<size; i++)
    {
        mok[i].num=i;

        for(int j=0; j<4; j++)
        {
            mok[i].kup2[j].knyg=j;
        }
    }

    delete [] mok;

    return 0;
}

[Disch]

So even if i delete something that hasn't been allocated with "new", it works.

You're wrong.

If you delete something that wasn't allocated with new, behavior is undefined. It might "work", or it might crash. There's no way to know.

One thing you can know is that you should never, ever, ever do it.



EDIT

in this example:

// lines 26,27:
        delete [] kupr;  // <- not allocated with new[]  BAD BAD BAD DON'T DO THIS EVER
        delete [] kup2;  // <- ditto


// line 49
delete [] mok;  // <- allocated with new[].  So this is Correct! 

[hakeris1010]

Youre right. That program crashed another time when called. So i decided to do checking before delete[]ing. At first i set all arrays to NULL. Then check.
if(arr!=NULL) delete [] arr;
But there's another problem:

arr=NULL;
if(arr!=NULL) delete [] arr;

But when i do the final destruction with checking if it is NULL, it starts deletion, even if the pointer was set to NULL!!! And it causes crash again!!! I don't know which magic processes makes the pointer not NULL. Any ideas?

[Disch]

deleting a null pointer is harmless. It effectively does nothing. It's totally safe.

int* foo = nullptr;
delete foo;  // <- safe.  Will not crash the program.  Will not do anything bad.
    // (will not do anything at all) 

But when i do the final destruction with checking if it is NULL, it starts deletion, even if the pointer was set to NULL!!! And it causes crash again!!! I don't know which magic processes makes the pointer not NULL. Any ideas?

You are overthinking this.

Null/not-null has absolutely nothing to do with it.


Does the pointer point to something allocated with new[]?
- If yes, then delete[] it.
- If no, then don't delete[] it.

It's that simple.

If you use new[] to allocate something, then you use delete[] to clean it up.
If you don't use new[], then you must not use delete[].


There's really nothing more to it.

[tipaye]

It would really help if you showed more code.

arr=NULL;
if(arr!=NULL) delete [] arr;

Where you do this matters, that's why I'm asking you to show more code. If for example you're doing this anywhere other than in your destructor, then your destructor is still going to do

delete [] kupr;
delete [] kup2;

anyway.

Assigning NULL to a pointer doesn't free the memory it points to, you've just leaked that memory most likely. Neither does deleting a pointer assign null to it. It's still holds the address of memory you no longer own.

You may want to assign NULL to a pointer after deleting it, as a way of checking in future whether it's been released or not.

...
int *x = new int[100];
...
delete [] x;
x = nullptr;
...

// in some other context that's aware of x
if (x)
{
// use x
}
else
{
// x is null, so we know it's been released, but this is up to the programmer to enforce, not the implementation
}

Have you considered using shared_ptr instead of this raw pointer business?

[hakeris1010]

That is the point, i set that pointer to null after deleting[] it.

[Disch]

That is the point, i set that pointer to null after deleting[] it.

Are you sure you are supposed to have deleted it?

I'm still not convinced you grasp the core concept here. Can you post your code?

[hakeris1010]

Thats my original code, where i use that problematic pointer. Im making a game which creates a world and writes it to file.

//world.h

class World
{
protected:
    long playerX=0;
    long playerY=0;

    int newChunkCount=0;

    Chunk *newChunks=NULL; //Thats the pointer
    Tile *tiles=NULL;
    Entity *entities=NULL;

public:
    World();
    ~World();

    int create(std::string name, uint8_t world_mode, uint8_t difficulty, long startX=0, long startY=0);

    void generateTiles(Chunk &chun);

    int saveChunksToFile(int mode=1);

    int generateChunks(long X, long Y, int howMany_X, int howMany_Y);
};

//world.cpp

World::~World()
{
    if(tiles) delete [] tiles;
    if(entities) delete [] entities;
    if(newChunks) delete [] newChunks;
}

int World::create(std::string name, uint8_t world_mode, uint8_t difficulty, long startX, long startY)
{
    //some code

    std::cout<<"Generating chunks \n";

    generateChunks(playerX, playerY, 4, 2);

    std::cout<<"saving chunks \n";

    if(saveChunksToFile()==-1) return -1;

    return 0;
}

int World::saveChunksToFile(int mode)
{
    std::cout<<"save chunks: opening ofstream \n";

    std::ofstream outp(filename2.c_str(), std::fstream::app);

    if(!outp.is_open()) return -1;

    std::cout<<"start loop \n";

    for(int i=0; i<newChunkCount; i++)
    {
        std::cout<<"outp header \n";

        outp<<" #chnk "<<newChunks[i].X<<" "<<newChunks[i].Y<<" "<<newChunks[i].how_many_entities<<" ";

        std::cout<<"posting chars \n";

        for(int j=0; j<64; j++)
        {
            outp<<char(newChunks[i].tiles[j].type)<<" ";
        }
        outp<<" #ents ";

        std::cout<<"entities \n";

        if(newChunks[i].how_many_entities>MAX_ENTITIES) newChunks[i].how_many_entities=MAX_ENTITIES;

        for(int j=0; j<newChunks[i].how_many_entities; j++)
        {
            outp<<newChunks[i].entities[j].active<<" ";
            outp<<char(newChunks[i].entities[j].Chunk_X)<<" ";
            outp<<char(newChunks[i].entities[j].Chunk_Y)<<" ";
            outp<<char(newChunks[i].entities[j].type)<<" ";
        }
        outp<<"\n ";

        std::cout<<"1 chunk posted \n";
    }
    std::cout<<"closing \n";

    outp.close();

    std::cout<<"size: "<<newChunkCount<<"\n";
    std::cout<<"deleting newChunks \n";

    newChunkCount=0;
    if(newChunks)
    {
        delete [] newChunks;
        newChunks=nullptr;
    }

    return 0;
}

int World::generateChunks(long X, long Y, int howMany_X, int howMany_Y)
{
    int ix, iy, i, am=0;

    Coordinate coord[4*howMany_X*howMany_Y];

    for(iy=Y-howMany_Y; iy<Y+howMany_Y; iy++)
    {
        for(ix=X-howMany_X; ix<X+howMany_X; ix++)
        {
            if(!checkChunk(ix,iy))
            {
                coord[am].x=ix;
                coord[am].y=iy;

                am++;
            }
        }
    }

    if(am==0) return 1;

    newChunkCount=am;

    if(newChunks) delete [] newChunks;

    newChunks=new Chunk[newChunkCount]; //creating that pointer

    for(i=0; i<am; i++)
    {
        newChunks[i].X=coord[i].x;
        newChunks[i].Y=coord[i].y;
        newChunks[i].how_many_entities=0;
        newChunks[i].number=i;

        generateTiles(newChunks[i]);
    }

    populateNewChunks();

    //if(saveChunksToFile()==-1) return -1;

    return 0;
}

[hakeris1010]

Wow! It works now. I just fixed the issue! There were other pointers which were causing this issue. Now its fine.