2

I wrote code very similar to this (although not identical) for a game I'm making. This has the same issue, so I'll provide it.

#include <iostream>

using namespace std;

class test {
public:
    int* arr;
    test(int n) {
        arr = new int[n];
        for(int i=0; i<=9; i++) {
            arr[i] = i;
        }
    }
    ~test() {
        delete[] arr;
    }
};

int main() {

    test obj = test(10);
    for(int i=0; i<=8; i++) {
        cout << obj.arr[i] << endl;
    }
    cout << obj.arr[9];
    obj.~test();
    return 0;

}

I know that I could rework my code and use the vector class to avoid this problem, but I want to actually understand why it's crashing. Removing the deconstructor call fixes the error, but that also introduces a memory leak, so I would prefer to just fix the deconstructor.

HDtCdr
  • 21
  • 2
  • 3
    No, the destructor gets called for you automatically. Calling it yourself, means it gets deleted twice. Having the destructor is enough to guarantee no leaks. – cigien Jun 01 '20 at 22:39
  • 1
    You don’t need to call the destructor explicitly. The compiler will set up the destructor call when the object goes out of scope. – James McLeod Jun 01 '20 at 22:39
  • this is part of the magic of c++, constructors and destructors are invoked at the correct time for you. – pm100 Jun 01 '20 at 22:49
  • Handy related reading: [What is meant by Resource Acquisition is Initialization (RAII)?](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii) and [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – user4581301 Jun 01 '20 at 22:51
  • @cigien "*Having the destructor is enough to guarantee no leaks*" - unless the class violates the [Rule of 3/5/0](https://en.cppreference.com/w/cpp/language/rule_of_three), that is (which this class is doing, but the code doesn't do anything that shows the violation happening). – Remy Lebeau Jun 01 '20 at 23:03
  • @RemyLebeau true, I misspoke. I *meant* only there's no need to call a (correctly written) destructor. – cigien Jun 01 '20 at 23:06

3 Answers3

5

This line is causing the error:

obj.~test(); // removing this will solve your problem

You shouldn't call the destructor. Once the object goes out of scope the destructor will get called.

halfer
  • 19,824
  • 17
  • 99
  • 186
bhristov
  • 3,137
  • 2
  • 10
  • 26
  • 1
    So should I never call a destructor, or should I just not do it here? – HDtCdr Jun 01 '20 at 22:43
  • 1
    Never call the destructor – Tas Jun 01 '20 at 22:44
  • 3
    Unless you are working with low-level std::allocator stuff, you should never-ever call the destructor yourself. – jvd Jun 01 '20 at 22:44
  • 1
    @HDtCdr you should never call the destructor. – bhristov Jun 01 '20 at 22:45
  • 1
    I've been programming in C++ for 30 years. I've never needed to explicitly call a destructor. But... I haven't made my own smart pointer yet, so maybe some day. – Eljay Jun 01 '20 at 22:49
  • @bhristov Good luck writing e.g. a custom `std::vector` replacement without calling a destructor manually. – HolyBlackCat Jun 01 '20 at 22:50
  • @HolyBlackCat, yes, there are specialized cases when we would want to do this but for someone not advanced with the language, I think it would be best to not consider calling the destructor a viable option. This is my personal experience from being a TA and working with students. – bhristov Jun 01 '20 at 22:54
  • @HDtCdr Anything that is constructed in *automatic* memory is destructed automatically for you. Anything constructed in *dynamic* memory via `new`/`new[]` must be destructed using `delete`/`delete[]`. Either way, the compiler handles the actual destructor call for you, DON'T call the destructor yourself directly. If you construct something in existing memory using `placement-new`, THEN AND ONLY THEN must you call the destructor directly before freeing the memory. – Remy Lebeau Jun 01 '20 at 23:16
4

You are explicitly calling the ~test() destructor on your obj variable, and then that same destructor will be implicitly called again when obj goes out of scope afterwards. Omitting the explicit destructor will NOT cause a memory leak. Let the compiler call the destructor for you. The ONLY time you can ever safely call a destructor explicitly is when the object being destructed was constructed using placement-new, which is not the case in your example.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
3

Just adding to the first answer that aptly notes that manual destructor invocation is errorneous here, I will also note that

class test {
public:
    int* arr;
    test(int n) {
        arr = new int[n];
        /* Redacted fro brevity */
    }
    ~test() {
        delete[] arr;
    }
};

has default copy-ctor and assignment operator. Those operators will crash the program and also leak the memory. Special copy-ctor, move-ctor, etc. should be defined in this context to provide proper memory management.

Even better, use std::vector that does that automatically.

jvd
  • 764
  • 4
  • 14