0

all. Suppose we have pointer to a struct, which has a member that is a dynamic array (among other members).

I can free all the objects but want your opinion on the best practice for this specific situation. See the code below, that compiles and runs without segmentation fault:

#include <iostream>

struct TestStruct
    {
    int a;  // And other members here
    int *b = new int[10];

    ~TestStruct()
        {
        }
    };

int main(void)
    {
    struct TestStruct *a_struct = new TestStruct();

    // Do something with the struct

    delete[] a_struct->b;
    delete a_struct;

    return 0;
    }

This way I am assuming the memory is returned properly. However, if I move any of these deletes to the destructor, there will be segfault. That is, if I move the array deletion to the destructor (delete[] a_struct->b;), it is no longer accessible because I deleted the pointer to the struct before (delete a_struct;), and vice-versa, and memory leak happens.

After reading this thread C++ free all memory used by struct, it is a bit inconclusive because most of the suggestions are taken for granted to work, but there is segfault in many of them.

I have simplified the problem since the array I will use is 3D. If it is not possible to free 100% of the memory in the destructor, then I am ready to use a method just to run the loops to free the array memory and the pointer to the struct. So I want to know your take on this specific situation.

Community
  • 1
  • 1
JayY
  • 109
  • 10

3 Answers3

3

Since you are using C++ and dynamic arrays, std::vector, std::array, or, e.g., std::unique_ptr are all better ways of handling this than working with new directly.

Richard
  • 56,349
  • 34
  • 180
  • 251
  • thanks. I have thought of using the safe arrays, but I am going to do a very long loop on a huge 3D array and using the access methods of these containers to write and retrieve individual elements will add a performance hit that I can't afford. I have tried before. – JayY Apr 07 '18 at 21:02
  • @JayY There is no performance hit using `std::vector` or `std::array`, they are basically syntactic sugar around a native array – Galik Apr 07 '18 at 21:19
  • I don't believe std::vector/std::array to be remarkably slower than a c-style array of you activate compiler optimizations. – JVApen Apr 07 '18 at 21:23
  • But in hundreds of millions of of iterations, the cumulative overhead is considerable. I'm working with immense seismic data. I've tried. – JayY Apr 07 '18 at 22:17
  • @JayY Have you measured it? There should not be any overhead. The code generated from accessing a `std::vector` element should be no different from the code generated from accessing a native array. – Galik Apr 07 '18 at 23:50
  • I, too, share the opinion that activating your compiler's optimizations should result in identical code and performance. Are you allocating 1D arrays and treating them like 3D arrays? This should give better performance than having arrays of arrays. – Richard Apr 07 '18 at 23:55
  • @JayY And if you are really worried about it, you can obtain the raw native array *first element* pointer from inside the `std::vector` and use that directly. But you don't need to because the compiler is smart enough to do that anyway. – Galik Apr 08 '18 at 00:00
  • @JayY See for yourself. The generated code is identical. With the `std::vector` there is one extra instruction at the beginning of the function to deal with the extra indirection. After that, the generated code to access it is identical to the raw array. https://godbolt.org/g/sxEhn5 (output lines 1-11). – Galik Apr 08 '18 at 00:08
  • Depending on application, perhaps the `std::vector` is not given aligned memory which is somehow preventing AVX optimizations. Usually there's a compiler flag that enforces this sort of thing. – Richard Apr 08 '18 at 00:13
0

The proper, RAII way to this would be:

struct TestStruct
{
    int a;  // And other members here
    int *b = new int[10];

    ~TestStruct()
    {
            delete[] b;
    }
};

int main(void)
{
    struct TestStruct *a_struct = new TestStruct();

    delete a_struct;

    return 0;
}

Proper design wouldn't allow multiple deletion by same pointer filed. If such risk exist, you can assign nullptr to pointer. deletion of null pointer is noop.

RAII (Resource acquisition is initialization) essentially boils down to: who allocated memory is one who deallocates.

Swift - Friday Pie
  • 12,777
  • 2
  • 19
  • 42
  • thanks for your reply. However, as I mentioned before, doing exactly this causes a segfault when in the destructor we try to delete[] b, because it is no longer accessible since delete a_struct already removed the reference to it. – JayY Apr 07 '18 at 20:52
  • 2
    The code posted as part of this answer is correct AFAICT, and running it on my machine (MacOS/X, clang++) produces no runtime errors. In C++ the TestStruct destructor is executed *before* the memory for the TestStruct is reclaimed, since to do otherwise would make it impossible for the destructor to free any resources. I think that if you are experiencing a segfault there must be some other reason. – Jeremy Friesner Apr 07 '18 at 21:02
  • @JayY only why it may segfault is that heap was corrupted before somehow. You didn't posted code that fails, so problem is not reproducable – Swift - Friday Pie Apr 07 '18 at 21:06
  • @Swift this very code you suggested, that I tried before asking the question here, causes the segfault. I tried with a few different compilers (g++, MSVC). In my mind, what is happening is: 1) program frees the memory of pointer a_struct; 2) destructor is called; 3) program tries to free the memory of the array a_struct->b, but there is no longer a reference to a_struct; 4) segfault. – JayY Apr 07 '18 at 22:24
  • @JayY you mean that exact copy of mmy program faults? I can attest that at least a dozen compilers don't do that, and oreder you describe is not matching rules of c++ language. See here: https://ideone.com/jRowfe – Swift - Friday Pie Apr 07 '18 at 22:32
  • @Swift I was using g++ in a MSYS2 environment, now I came to the free Mobile C/C++ compiler for IOS, also tried with Clang, and the delete[] b in the destructor will NOT segfault. You suggested something corrupt, which I will be able to attest after restarting the PC, but honestly, I don't know. If it works, it works, so I will mark as solution all the posts that suggested delete[] b in the destructor. Thanks for everyone's patience. – JayY Apr 07 '18 at 22:54
0

In the destructor, delete only dynamically allocated members, but not the object itself (this is done in the course of destruction).

So the following code should be fine:

struct TestStruct
{
  int a;  // And other members here
  int *b = new int[10];

  ~TestStruct() {
     delete[] b;
  }
};

int main(void)
{
  struct TestStruct *a_struct = new TestStruct();

  // Do something with the struct

  delete a_struct;

  return 0;

}
Stephan Lechner
  • 34,891
  • 4
  • 35
  • 58
  • 1
    LOL, in same second... also, technically you can `delete this`, but it would lead to recursive call of destructor if it is done from destructor. – Swift - Friday Pie Apr 07 '18 at 20:27
  • @Stephan, thanks, but it is the problem I mentioned in my comment to Swift above. I have tried this before asking the question. After delete a_struct, the delete[] b in the destructor causes a segfault (a_struct no longer accessible). – JayY Apr 07 '18 at 20:54
  • @JayY: `delete a_struct` actually triggers the destructor, an in the destructor, `this` is always accessible (otherwise any destructor code would not make much sense). Are you sure you removed the `delete[] a_struct->b` in `main`? – Stephan Lechner Apr 07 '18 at 20:57
  • @StephanLechner yes, I have tried this before, but it will segfault on any delete command in the destructor if I had already deleted something in main(). I have the compiler ready for any suggestion people make here. – JayY Apr 07 '18 at 21:08
  • @JayY Put the code that causes a segfault in the question. – Galik Apr 07 '18 at 21:22
  • @Galik if you move the delete[] a_struct->b to the destructor, while leaving the delete a_struct in the main code, this will cause the segfault. – JayY Apr 07 '18 at 22:19
  • 1
    @JayY I don't see why the posted code should segfault. Have you tried it with *exactly* that code? – Galik Apr 07 '18 at 22:30
  • you cant put `delete[] a_struct->b` into destructor, you have to use `delete[] b;` – Swift - Friday Pie Apr 07 '18 at 22:37
  • @JayY I suspect I know what your problem is because this code here will not work in *all* situations. That is why I wanted you to post the code that causes the segfault, because I believe it will expose your *real* problem (and the problem with all these solutions). – Galik Apr 07 '18 at 22:41
  • @Swift exactly, delete[] b, my bad. I have no problem keeping the code as it is, deleting both the array and the struct pointer right after using the struct, limiting the scope with curly braces. Unless I declare the struct object as a normal variable instead of a pointer, and then I don't have to worry about deleting it. – JayY Apr 07 '18 at 22:48
  • @Galik I was doing this on a g++ in MSYS2 (Windows), which I don't believe has anything to do. Then I repeated the code, moving delete[] b to the destructor, in Mobile C/C++ compiler for IOS, and it WORKED. Then I tried with Clang, it WORKED. I will restart this PC and try again in MSYS2. But I want to thank your help, and everyone else's, and mark the solution. – JayY Apr 07 '18 at 22:57
  • @StephanLechner I am upvoting your solution (in fact, upvoting other posts), though I can only mark 1 as solution. So I want to thank you and everyone else all the same. – JayY Apr 07 '18 at 22:59
  • @JayY Well you may want to consider this: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three – Galik Apr 07 '18 at 23:46