why this delete still lead to runtime exception even though I set pointer as NULL after delete

Question

// CplusTest20161027.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"


#include <iostream>

using namespace std;

class Line {
public:
    int getLength(void);
    Line(int len);             // simple constructor
                               // Line( const Line &obj);  // copy constructor
    ~Line();                     // destructor

private:
    int *ptr;
};

// Member functions definitions including constructor
Line::Line(int len) {
    cout << "Normal constructor allocating ptr" << endl;

    // allocate memory for the pointer;
    ptr = new int;
    *ptr = len;
}

//Line::Line(const Line &obj) {
//   cout << "Copy constructor allocating ptr." << endl;
//   ptr = new int;
//   *ptr = *obj.ptr; // copy the value
//}

Line::~Line(void) {
    cout << "Freeing memory!" << endl;
    if (ptr)
    {
        delete ptr;
        ptr = NULL;
    }
}

int Line::getLength(void) {
    return *ptr;
}

void display(Line obj) {
    cout << "Length of line : " << obj.getLength() << endl;
}

// Main function for the program
int main() {
    Line line(10);

    display(line);

    return 0;
}

in above code, I commented the copy constrcture to test what happened. and I found the pointer would be deleted twice so it is an exception. So I decide to check if the pointer should be deleted, by setting prt=NULL after it was delete. But still, ptr will be delete TWICE. Why? can somebody explain the logic to me? please help me figure out what is wrong? the output is: Normal constructor allocating ptr Length of line : 10 Freeing memory! Freeing memory!

with an exception on heap.

`ptr = new int;` - Are you sure you want to allocate memory for just one int? In other words, potentially wasting 8 bytes for a pointer to store a value that fits into 4? And having to deal with cleanup and resource management? Anyway, this is a duplicate: [What is The Rule of Three?](http://stackoverflow.com/q/4172722/1889329) — IInspectable, Oct 27 '16 at 10:54
@IInspectable: I think the OP's code is just for _learning_ purposes of `new` and `delete`; in production code allocating an `int` on the heap like done there doesn't make sense (unless they want a semantics of _optional_ value, in which `nullptr` means something like _"no value"_, and even in this case just adding a `bool` data member instead of dynamic allocation might be better), but can make sense for learning purposes when _experimenting_ with dynamic memory allocation. — Mr.C64, Oct 27 '16 at 11:18
Setting the pointer to NULL in the destructor is pointless, since the object is going away. Also, you don't need to test for NULL before deleting the object; `delete` handles NULL pointers appropriately. — Pete Becker, Oct 27 '16 at 13:57

Mr.C64 · Answer 1 · 2016-10-27T10:59:05.547

I think the problem is that you pass the Line object by value to the display function:

void display(Line obj) {

and then at the call site:

Line line(10);
display(line);

This makes a copy of line, and the default copy constructor will do a member-wise copy of Line's members, i.e. its int* ptr pointer.

So, there are two destructor calls: one for the original line object, and one for the copy, and you get double destruction.

The first destructor call properly deletes the initially allocated heap memory.
But the second destructor call attempts to delete memory that was already deleted in the previous call.

You should either ban copy, or implement a proper copy semantics for your Line class.

To ban copy, you can use the C++11 =delete syntax for both the copy constructor and copy assignment operator=, e.g.:

class Line {
 public:
  ... 
  // Ban copy
  Line(const Line&) = delete;
  Line& operator=(const Line&) = delete;
};

If you ban copies, you can still pass Line objects e.g. via const& to functions:

void display(Line const& obj) {

P.S.
delete works just fine with nullptr (which is the modern C++11 version of NULL in your code) pointers, so you don't need the redundant if (ptr) check before calling delete.

But the second destructor call attempts to delete memory that was already deleted in the previous call. — Penny, Oct 27 '16 at 11:16
But the second destructor call attempts to delete memory that was already deleted in the previous call.---If I do NOT write any explicit code on COPY constructure. and I know it is possible ptr would be deleted twice. How should I avoid it? by checking delete ptr in someway? is it possible? — Penny, Oct 27 '16 at 11:23
You should avoid it as I already wrote in my answer, i.e. please fix your class, either _explicitly banning_ copy semantics e.g. with `=delete`, or by properly _implementing_ copy semantics. — Mr.C64, Oct 27 '16 at 11:25

why this delete still lead to runtime exception even though I set pointer as NULL after delete

1 Answers1