I have a recursive function defined sort of like this:

void routine(int* old_arr, int k, int depth) {
    if (depth == 15) return;
    int* new_arr = new int[size];
    // some operation that builds new_arr values out of old_arr values
    delete [] old_arr; // if i dont need old_arr anymore, can I just do this?
    for (int i = 0; i < 3; i++) {
        routine(new_arr, k);
    }
    return;
}

If you'll notice, on the 2nd call of the for loop there will be a double free.

See this as a depth first search along a ternary tree. the function argument old_arr is used to fill values of a new array called new_arr. Notice old_arr is only needed to compute the values at the next depth. If I don't have any sort of free routine, I will have a lot of this pointers floating around consuming memory and not doing anything. What is a good design around this?

I've considered using something like a shared_ptr<int>, and replacing the delete[] with

shared_ptr<int> new_arr(new int[size]);
// operators to initialize new_arr with old_arr
if (old_arr.use_count() == 1) old_arr.reset()

Is this the right idea? Edit: actually that won't work either.

I have a recursive function defined sort of like this:

void routine(int* old_arr, int k, int depth) {
    if (depth == 15) return;
    int* new_arr = new int[size];
    // some operation that builds new_arr values out of old_arr values
    delete [] old_arr; // if i dont need old_arr anymore, can I just do this?
    for (int i = 0; i < 3; i++) {
        routine(new_arr, k);
    }
    return;
}

If you'll notice, on the 2nd call of the for loop there will be a double free.

See this as a depth first search along a ternary tree. the function argument old_arr is used to fill values of a new array called new_arr. Notice old_arr is only needed to compute the values at the next depth. If I don't have any sort of free routine, I will have a lot of this pointers floating around consuming memory and not doing anything. What is a good design around this?

I've considered using something like a shared_ptr<int>, and replacing the delete[] with

shared_ptr<int> new_arr(new int[size]);
// operators to initialize new_arr with old_arr
if (old_arr.use_count() == 1) old_arr.reset()

Is this the right idea? Edit: actually that won't work either.

• c++
• recursion
• memory-management
• memory-leaks
• c++20

Share Improve this question Follow asked Jan 31 at 9:39 BootsBoots 13122 bronze badges 13

• 4 Use std::vector instead of raw new. – Jarod42 Commented Jan 31 at 9:50
• 1 Sorry Boots but you need all of the allocations anyway. Each call to routine() can't know whether old_arr is still needed, only its caller knows. And the caller must keep it alive for all three calls to routine(). – Charles Savoie Commented Jan 31 at 9:59
• 1 BTW, typo, as routine need 3 parameters, but only 2 provided. Unused i is strange. should it be routine(new_arr, i, depth + 1);? – Jarod42 Commented Jan 31 at 10:03
• 1 And that's where the source of your confusion lies. You can test it with a global counter. Make it increment at the start of a function (=allocation) and decrement before return (=deallocation), calculate its maximum over the process (=max live objects at any moment in time). Did you get a large number? – teapot418 Commented Jan 31 at 10:07
• 1 @Boots But each node also frees its memory when it is done. Look at it another way, say you're in the middle of routine at some depth depth. How many different new_arr arrays are there? Since they are all freed at the end of routine(), there can only be one per active call to routine(), i.e., one for each depth, or one for each level of the call stack. – Charles Savoie Commented Jan 31 at 10:08

 |  Show 8 more comments

1 Answer 1

Sorted by: Reset to default 3

In a nutshell, yes, it sounds like you need shared_ptr. But you have some strangeness in your recursion -- the caller needs new_arr to stay alive, because it calls routine() three times, so it shouldn't be up to routine() to free it, ever. Let the caller free it when needed. It can't work any other way.

Use make_shared instead of new, it prevents mistakes with new and ensures one heap allocation instead of two (one for the array and one for the shared pointer control block). Also, never use use_count() except for debugging. Here's some amended code:

void routine(shared_ptr<int[]> const& old_arr, int k, int depth) 
{
   if (depth == 15) return;
   auto new_arr = make_shared<int[]>(size);
   // some operation that builds new_arr values out of old_arr values
   for (int i = 0; i < 3; i++) {
       routine(new_arr, k);
   }
}