0

I'm creating a tree of dynamic objects. The Node class has a vector to store the child nodes, among the other class variables:

std::vector<Node*> child;

The class destructor deletes all the dynamically allocated class variables, and then deletes the child nodes:

~Node() {
    //Deleting the other variables
                .
                .
                .

    //Deleting the child nodes
    for(int i = 0; i < child.size(); i++) {
        delete child[i];
    }
}

My class has a method that creates a tree of a given height, in which the current node is the root node:

void createTree(int height) {
    if(height == 0) {
        return;
    }
    for(int i = 0; i < numberOfChildNodes; i++) {
        child.push_back(new Node());
        child[i]->createTree(height - 1);
    }
}

This class has another method where I create a tree with height = 3, then I delete the entire tree and create another one with height = 4, then I delete the entire tree and create one with height = 5, and so on, until a memory limit is reached:

void highestTreePossible() {
    int i, height = 3;
    struct sysinfo memInfo;
    while(true) {
        createTree(height);
        sysinfo (&memInfo);
        if(memInfo.freeram > limit) {
            std::cout << "Highest tree possible: height = " << height;
            break;
        }
        for(i = 0; i < child.size(); i++) {
            delete child[i];
        }
        child.clear();
        height++;
    }
    for(i = 0; i < child.size(); i++) {
        delete child[i];
    }
    child.clear();
}

The problem is, when I check the memory after running the method highestTreePossible(), there's a lot of memory allocated, which isn't supposed to happen, because I deleted everything. Why is my code leaking memory?

MrPontes
  • 29
  • 5
  • 6
    *when I check the memory after running the method* -- How do you do this check? – PaulMcKenzie May 19 '20 at 14:04
  • 2
    Please use RAII pattern (`std::unique_ptr`). – Marek R May 19 '20 at 14:06
  • @PaulMcKenzie I use struct sysinfo memInfo; sysinfo (&meminfo); std::cout << "Free memory: " << memInfo.freeram; – MrPontes May 19 '20 at 14:07
  • 2
    If this "meminfo" reports OS memory, then your test is not valid. The C++ heap manager is smart enough to hold onto allocated memory. A better test is if you continually call this function, say 1000 times in a loop of some type. If you did that, does "meminfo" report leaked memory increasing at a high rate? – PaulMcKenzie May 19 '20 at 14:08
  • 5
    The C++ memory manager does not necessarily return memory to the OS, since there is a very high probability that the program is going to want more and allocating from the OS is costly. – molbdnilo May 19 '20 at 14:08
  • Memory management on a modern OS is waaay more complicated than this. It's not 1930 any more ;) – Asteroids With Wings May 19 '20 at 14:10
  • @molbdnilo If that's the case, then I will run out of RAM very quickly. I need the memory manager to return the memory so I don't run out of RAM. What can I do? – MrPontes May 19 '20 at 14:11
  • Use valgrind to check for memory leaks. But even if you find with valgrind that there is no leak, consider changing you vector of `Node` pointers to a vector of `unique_ptr`. There is no reason to do the memory management yourself. – darcamo May 19 '20 at 14:11
  • 1
    You won't run out of RAM very quickly, or at all. Your computer is smart, trust it :) – Asteroids With Wings May 19 '20 at 14:11
  • 1
    @MrPontes -- Before concluding you will run out of RAM, do the test I suggested. If the memory stays level after the first call, and then doesn't increase when that function is called a thousand times, then there is no suspected leak. – PaulMcKenzie May 19 '20 at 14:12
  • @MrPontes The manager will return memory when it believes that it's a good idea. It knows what it's doing and is generally much better at managing memory than a human is. – molbdnilo May 19 '20 at 14:12
  • @PaulMcKenzie Ok I will try that – MrPontes May 19 '20 at 14:15
  • @PaulMcKenzie You are right. This test returns the OS memory, so it's not valid. But the thing is, I did your test and I ran the highestTreePossible() method a few times in a row. It ran fine the first 3 times, but on the 4th time, the computer became so slow that I couldn't even switch tabs. I'm pretty sure I ran out of RAM. – MrPontes May 19 '20 at 14:41
  • Maybe your code has bugs, and has nothing to do with allocation of memory. – PaulMcKenzie May 19 '20 at 15:05

2 Answers2

2

It's not leaking; you're not using a valid test for this kind of thing. Modern operating systems have complex memory management, and you may observe a process "holding on" to more memory than you think it needs at any given time. Rest assured, it is available to the rest of the system when required.

If you're concerned about memory, you need to observe a constant, consistent rise in consumption for your process over a significant period of time, or hook into the actual allocators used by your program itself. A great way to do this is using a diagnostic tool like massif, which ships with Valgrind (if you're on a compatible system). There are some great ways to visualise that.

Asteroids With Wings
  • 17,071
  • 2
  • 21
  • 35
0

If you're on a Linux system, you can try to use valgrind to reliably check if your code has any memory leaks.

e.g.  $valgrind --leak-check=full ./node_test

So to answer the question "Why is my code leaking memory?"

There is no memory leak in your code as I tested/checked it with valgrind report (e.g. "All heap blocks were freed -- no leaks are possible"). However, I seem to notice some problem in the line containing the exit condition if(memInfo.freeram > limit) since memInfo.freeram value is expected to be decreasing as the tree grows. And that your goal is to keep the tree growing (hence memInfo.freeram will also be shrinking) "until a memory limit is reached" as you've mentioned above or "until the memInfo.freeram falls below a certain limit" as I rephrased it.

So I expect that the correct exit condition should supposedly be the opposite if(memInfo.freeram < limit).

Hope this helps.

PS. It's also a good practice to use smart pointers (e.g. std::unique_ptr, std::shared_ptr) to avoid memory leaks.

Jeune Prime Origines
  • 239
  • 2
  • 5
  • When I was writing the code in this question, I accidentally typed '>' instead of '<'. But it's correct in my original code. Thanks for pointing that out. – MrPontes May 19 '20 at 16:54