Array not correctly deleted?

[ForeignCurs]

I use the following methods in a for-loop:

bool **** __fastcall createMArray(int x, int y, int z, int n) {
        bool **** done = new bool***[x];
        for(int o = 0; o < x; o++) {
                done[o] = new bool**[y];
                for(int j = 0; j < y; j++) {
                        done[o][j] = new bool*[z];
                        for(int u = 0; u < z; u++) {
                                done[o][j][u] = new bool[n];
                                fill_n(done[o][j][u],n,false);
                        }
                }
        }
        return done;
}

void __fastcall deleteMArray(bool **** done, int x, int y, int z) {
        for(int i = 0; i < x; i++) {
                for(int j = 0; j < y; j++) {
                        for(int u = 0; u < z; u++)
                                delete [] done[i][j][u];
                        delete [] done[i][j];
                }
                delete [] done[i];
        }
        delete [] done;
}

By doing:

for(int i = 0; i < 10; i++) {
    bool ****done = createMArray(x,y,z,n);
    do_something_leak_free();
    deleteMArray(done,x,y,z);
}

Turns out the delete attempt doesn't work. At all. Memory does not get freed and after some loops the program crashes when no memory is left. I have 2GB of memory that is available when running the program, which is more than the array I try to make. In practice, the array seems to take 200MB in a single loop. Memory leak detection programs also indicate that the array is the issue (taking 98% of the memory), not something else.

What do I do wrong?

[Lynx876]

I'm pretty sure that you can't just delete a single element:
delete[] done[ i ];

You just call: delete[] done;

[ForeignCurs]

Does that matter seeing I do call
delete [] done;
at the end anyway?

I was thinking maybe the entire array of pointers gets deleted each time, but the content it refers to (a lot of booleans) isn't?

[S6k9GNh0]

ForeignCurs, your use of really large multi-dimensional arrays is a design flaw. I suggest you just not do that.

[ForeignCurs]

Except that there is no other way to my knowledge.

I want instant access to read and set a boolean for XxYxZxN points. Other suggestions are welcome ofc.

If there is no other instant access method, then still why doesn't this work?

[Gaminic]

I've tested your code in a separate project and it works perfectly. All I can think of is that your do_something_leak_free() changes the values of x, y or z.

(Here's the code I used to check:

#include <algorithm>
#define _CRTDBG_MAP_ALLOC
#include <iostream>
#include <crtdbg.h>
#ifdef _DEBUG
#define DEBUG_NEW new(_NORMAL_BLOCK, __FILE__, __LINE__)
#define new DEBUG_NEW
#endif
// Declaration of your two functions, straight copy-paste from your post.
int main() {
	bool ****test = createMArray(5, 10, 15, 20);
	test[1][3][5][6] = true;
	deleteMArray(test, 5, 10, 15);
	_CrtDumpMemoryLeaks();
	return 0;
}

[ForeignCurs]

Debugging shows that x, y and z are still unchanged.

If you could try your program in a loop please, with sizes ~ 100x100x100x20?

[Gaminic]

bool ****test = createMArray(100, 100, 100, 20);
test[1][3][5][6] = true;
deleteMArray(test, 100, 100, 100);

Works perfectly.

[edit]

Why is your array taking up 200MB? A bool is 1byte, 100³*20 = 20kk byte ~= 20MB.

[ForeignCurs]

I love it when the same code works at one place and doesn't at another...

All right, apparently my machine has a different idea of what delete means, so are there perhaps any suggestions as to other constant access collections?

I was thinking about a map, but then I'd have to make a structure to embody (x,y,z,n)...

[S6k9GNh0]

Why are you using __fastcall?

Also, check this out...

bool * createMArray(int x, int y, int z, int n) {
     bool * done = new bool[x*y*z*n]; //Same allocation as before.
     return done;
}

void deleteMArray(bool * done, int x, int y, int z) {
        delete[] done;
}

Tada. Only difference is you don't use the array the same way you did before.

[ForeignCurs]

Nice :) Gonna check that out and edit in this post ;)

Edit: __fastcall is the result of some legacy code I was improving upon... Removing it did not change anything.

Edit2: Implementing it with the above solution did not free the memory either...

[S6k9GNh0]

__fastcall changes the calling convention and slightly modifies how the function is called (usually recognized through symbol mangling). It often doesn't have much performance difference but makes it difficult to interface to other languages.

[ForeignCurs]

As it turns out my memory leak detection finds that:

bool [] exists 1 time in memory with 39.000.000 bytes (which clearly isn't the problem).

This is odd since bool [] is constructed 4 times, so I'd reckon the deletion works but the detection still finds an old shade of bool[] in the memory?

[Gaminic]

I'd say one (most likely but not necessarily the last) of them isn't deleted. We can't help you without more information. The snippets you posted work perfectly.

Have you tried starting a new project and just copying the code I used? If that works, you're certain it's the other code in your project that's messing things up.

I'm also slightly confused to the 39mil bytes. It should be 20mil, no more, no less.

[ForeignCurs]

I'd say one (most likely but not necessarily the last) of them isn't deleted. We can't help you without more information. The snippets you posted work perfectly.

As it turns out the detector only detects a single occurence of the array, while if it weren't deleted it would be at least 4 occurances. So I guess that last array it finds is just some messy thing in the detector itself.

Have you tried starting a new project and just copying the code I used? If that works, you're certain it's the other code in your project that's messing things up.

Indeed, I discovered some memory leaks I hadn't covered yet... I expected the legacy code to be leak free. Not so.

I'm also slightly confused to the 39mil bytes. It should be 20mil, no more, no less.

Only 100x100x100 is rigid. The xN is actually quite variable over the run and was apparently 39 in the last run.

[Gaminic]

If it finds a single occurance of 39mil bytes, it's obvious your program is exiting before the last delete.

[S6k9GNh0]

Really, this is flaw in your program design. Stop using 4-dimensional dynamically allocated arrays. Not only is that sloppy and confusing, it's causing bugs in your program. That means it's a good time to search for alternative methods.

[Gaminic]

How is it more confusing than trying to access elements by calculating a single-dimension index from 4 coordinates? Code-wise, dynamic MD arrays can be confusing, but I think most of the problems are in the logic of an algorithm, not the form data is saved in.

[ForeignCurs]

I'm wholly open to suggestions that are also constant in time to access.

Currently I employ the single-dimension index because it makes the code looks less bloated.

Besides, I really am confident that the single occurance is just a ghost, because if I put an outer loop around the current code, it stays at the same memory level, implying it's deleting everything quite nicely.