Destructor causes memory exception

[colt]

Hello. In the program I am creating, I created a destructor in a certain class, one that calls delete to remove two dynamic allocated array from two different user defined types. When I run the program, this shows up in the command line:

*** glibc detected *** ./ray-tracer: free(): invalid next size (fast): 0x00000000025c4010 ***

I didn't note anything wrong wit the destructor

~Scene () { delete sphere_array; delete light_array;}

The memory allocation code it is:

Scene (int spheres, int lights) : n_spheres (spheres), n_lights (lights) {
         sphere_array = new Sphere [n_spheres];
         light_array = new Vec3f [n_lights];
      }

sphere array and light array codes:

      Sphere* sphere_array;
      Vec3f* light_array;

This is caused by the destructor presence only. If I replace by:

void delel () { delete sphere_array; delete light_array;}

The problem vanishes. So I would like to understand what causes this issue and the best course of action to follow, if it is to use a function like the one above

[Peter87]

Since they are arrays you should use delete[].

delete[] sphere_array;
delete[] light_array;

[colt]

Hi Peter87. Unfortunately it didn't solve it. What I said above about the "delel" function is incorrect, problem occurs in both cases.

[seeplus]

What's the values of sphere_array and light_array when allocated and what's their values in the destructor or in delel()? It could be that the value(s) are being modified somewhere. Or you have a problem with the copy constructor or operator=() member functions. If these haven't been provided then if they are used then the compiler generated ones will be wrong as they do a shallow rather than a deep copy. If not provided, then either provide them to do a deep copy, or mark them as =delete. In this case if they are used then the compiler will provide compile time errors.

If you have already provided a copy constructor and operator=(), then post the code so that we can have a look.

[Peter87]

Have you implemented (or disabled) the copy constructor and assignment operators?

Otherwise that'll cause problems if you, intentionally or accidentally, copied one of your Scene objects.

If that's not it, then I suggest you create a "minimal reproducible example".
https://en.wikipedia.org/wiki/Minimal_reproducible_example
In the process you're likely to find the problem yourself, but if you don't then you can post it and we can have a look and probably be able to tell you what is wrong.

[jonnin]

set them to null:
void delel () { delete[] sphere_array; delete[] light_array; sphere_array=light_array= nullptr;}

if that works, its likely you are calling the dtor more than once somehow; there are a variety of ways that can happen.

[keskiverto]

Is there a particular reason why you can't write:

class Scene {
    std::vector<Sphere> spheres;
    std::vector<Vec3f>  lights;
public:
    ~Scene () = default;

    Scene (int spheres, int lights)
     : spheres(spheres), lights(lights)
    {}

    // more interface
};

[colt]

Ok, problem fortunately (or not) solved. I was trying to do a minimal reproducible example. When testing it, I noticed the problem had vanished. Yesterday when working on other parts of the code, I changed something that fixed the problem. I believe it was when I was working on constructors and related areas, although I abandoned nearly all changes on that area As I don't know the reason, it might mean that the problem will surface again in the future. But for the time being, it's solved.

sphere_array=light_array= nullptr;}

This doesn't work on my system for for some reason "not declared"

I would have to use: sphere_array=NULL, light_array=NULL

In the end, using delete sphere_array or delete [] sphere_array made no difference.

Is there a particular reason why you can't write:

Yes. That will be too slow for that part of the code. I will use it when dealing with file reading, when speed isn't crucial.

[seeplus]

If nullptr isn't defined, then you're using C++98! What compiler are you using? I really, really, strongly advise an update to the compiler to a modern version. The current C++ standard is C++20 - with C++23 due soon.

[keskiverto]

For example, older GCC versions were in C++98 mode by default, even though they had support for more recent C++ standards. One had to use -std= option to specify the mode.


You have shown that code with std::vector is slow? Is that still true with recent compilers and language version, which might heve better optimization?

Is the manually written memory management actually significantly faster (now, when it presumably is correct)?

[colt]

If nullptr isn't defined, then you're using C++98! What compiler are you using? I really, really, strongly advise an update to the compiler to a modern version. The current C++ standard is C++20 - with C++23 due soon.

I use g++ 4.9.4, which is from 2015, not that old I guess. An improvement over the 4.4.3 that I used not too long ago, I suppose.
As I don't use any c++11 feature (at least for now), it doesn't fell that important. On the other hand, gcc takes an eternity to build, and a long time to extract from the compressed file.

You have shown that code with std::vector is slow? Is that still true with recent compilers and language version, which might have better optimization?

Is the manually written memory management actually significantly faster (now, when it presumably is correct)?

In my previous program (that I am actually rebuilding from the ground now) I detected up to 6% performance hit (if my memory serves me well) when using it instead an fixed size array. It was probably on g++ 4.4.3.

Anyway, when I reach the point that performance comparisons can be made, I will do so, especially when the most interesting use cases shows up, with 4 to 6 digits of elements in the array/vectors.

[keskiverto]

C++11 was quite a jump and apparently C++20 will be too (once compiler support improves). An important question is whether you have "legacy" code that does not conform to C++11 (and later). If not, then it should be a no-brainer to switch. You obviously do not need to start using any of the conveniences that "modern" C++ has.

On the other hand, gcc takes an eternity to build, and a long time to extract from the compressed file.

On what platform are you on?

(I might be spoiled by Enterprise Linux that has prebuilt binaries in package repo: RHEL 7 was released in 2014 and has GCC 4.8.5, but also newer GCC versions -- up to 11. RHEL "4.8.5" had c++11 and c++1y.)

[seeplus]

An important question is whether you have "legacy" code that does not conform to C++11 (and later)

I'd guess that there is. When we moved from ANSI C++ to C++98 to C++11 to C++14 to C++17 et al (with new VS versions), we had quite a lot of work to do to get the code base to compile. Early MS VS was quite lax with the standard and allowed things it shouldn't. I seem to remember the conversions taking several months...