Deletion of a class pointer in release build causes memory issues

Question

MFC application using /clr written in VS2010. Multi-threaded DLL (/MD) run-time library. Problem occurs when I switch the Preprocessor Definition of NDEBUG to _DEBUG. NDEBUG disables the assertion which pops up when _DEBUG is defined. Am I doing something wrong here in managing the creation and deletion of a class pointer?

Once I make the switch from NDEBUG to _DEBUG, I get a "_Block_Type_Is_Valid (pHead->nBlockUse)" assertion failed error at run time.

Class A : "A.h"

#include "B.h"

class A
{
public:
    A(void);
    ~A(void);

    A(const A&);
    A& operator=(const A&);

    B* p_B;

};

Class A: "A.cpp"

#include "StdAfx.h"
#include "A.h"

A::A(void)
{
    p_B = new B();
}

A::~A(void)
{
    delete p_B;
}

// 1. copy constructor
A::A(const A& that)
{
     p_B = new B(); 
    *p_B = *that.p_B;
}

// 2. copy assignment operator
A& A::operator=(const A& that)
{
    *p_B = *that.p_B;
    return *this;
}

Class B: "B.h"

class B
{
public:
    B(void);
    ~B(void);

    B(const B&);
    B& operator=(const B&);
};

Class B: "B.cpp"

#include "StdAfx.h"
#include "B.h"

B::B(void) { }
B::~B(void) { }

// 1. copy constructor
B::B(const B& that)
{
}

// 2. copy assignment operator
B& B::operator=(const B& that)
{
    return *this;
}

ModalDlg.cpp (Where an object of class A is instantiated)

BOOL CTestingReleaseBuildDlg::OnInitDialog()
{
    CDialogEx::OnInitDialog();

    A a;

    // Set the icon for this dialog.  The framework does this automatically
    //  when the application's main window is not a dialog
    SetIcon(m_hIcon, TRUE);         // Set big icon
    SetIcon(m_hIcon, FALSE);        // Set small icon

    // TODO: Add extra initialization here

    return TRUE;  // return TRUE  unless you set the focus to a control
}

Then I simply instantiate the A class in my MFC dialog and that causes the assertion to fail. My question is, "Am I doing something wrong here in the creation and deletion of a class pointer?" The assertion specifically fails in the "delete p_B" instruction of class A's destructor.

EDIT: I instantiate the A class with BOOL CMyMFCClassDLG::OnInitDialog() { ... A a; ...}

EDIT2: I defined the copy constructors and the copy assignment operators for both A and B classes. They are never invoked.

EDIT3: Worth mentioning that if I remove the delete p_B; statement in A's destructor then no more assertion failure.

EDIT4: The program runs fine when in Debug mode with /MDd and _DEBUG defined. The assertion fails when I run in Release mode with /MD and _DEBUG. Which I think might be causing the issue since /MD should probably be ran with NDEBUG.

EDIT5: I updated the code as suggested by @Christophe and inserted the function of where an object of Class A is instantiated. I dont want to copy/paste the rest of the Modal Dialog application but you can replicate the exact code by starting a new Modal Dialog based MFC application in VS2010 and change the project configuration to work with /CLR mode, set the Run-Time Library to /MD and include the _DEBUG keyword in pre-processor definitions field.

EDIT6: Link to project https://drive.google.com/drive/folders/1q0n9c6yMZ2ZKnakH6Z5NbVeGsAWfUAc1?usp=sharing

Possible duplicate of [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) and https://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11 — Richard Critten, Jan 16 '18 at 16:42
How do you instantiate it ? If you use `operator = (...)` you're in trouble. — Sid S, Jan 16 '18 at 17:14
@SidS Valid point. I'm trying to make it work as it was previously designed this way. — Anton Savelyev, Jan 16 '18 at 17:20
So does this error happen once I attempt to delete p_B which becomes a dangling pointer after the destructor of B is called? — Anton Savelyev, Jan 16 '18 at 17:24
You didn't answer the question about how you instantiate it. — Sid S, Jan 16 '18 at 17:41
@SidS Updated question body to show how I instantiate it. Basically its `A a;` — Anton Savelyev, Jan 16 '18 at 17:46
The most likely cause of the assert failure is that you delete the same object more than once. This could be caused by making a copy of `a`, for instance if you instantiated it with `A a = A();` or passed `a` by value to a function or ... — Sid S, Jan 16 '18 at 17:54
Yes, if I take out the `delete p_b` line in the destructor then I don't get the assertion failure. But I don't delete this pointer or the `A a` object anywhere else. What I wrote in the question body is all the code that is written. The flow of events that happen with the current code is that at run time after initialization it enters A's destructor, then B's destructor but fails to go past the delete operation in A's destructor. — Anton Savelyev, Jan 16 '18 at 18:07
It's still unclear, what code you are running. Until we can see a [mcve], this question cannot be answered. — IInspectable, Jan 16 '18 at 23:02

Christophe · Answer 1 · 2018-01-16T23:52:33.057

1

Without copy constructor and assignment operators, p_B gets cloned from its original A object. So the first of the two objects that gets destroyed will delete p_B, and the second object will try to delete the already deleted pointer, which is UB.

In your edit, you have defined the missing elements. The problem with your copy constructor, it that it does nothing. So unfortunately, the p_B pointer of the copied object might be invalid. You need to complete those member functions:

// 1. copy constructor
A::A(const A& that)
{
    p_B = new B(); 
    *p_B = *that->p_B;
}

For the copy constructor, assuming that you guarantee that p_B will always point to a valid B object, and assuming there's no risk of slicing:

// 2. copy assignment operator
A& A::operator=(const A& that)
{
    *p_B = *that.p_B;        
    return *this;
}

If you think you don't need a copy constructor nor an assignment operator, in order to ensure that the rule of 3 is respected, you could alternatively declare them as deleted:

A(const A&) = delete;
A& operator=(const A&) = delete;

If your code would accidentally use them, the compiler would complain instead of generating the code and the problems mentioned above.

Finally, you instantiate A in an init function. But with your very short snippet, it appears that it's a local object of this function. So it gets destroyed as soon as you leave that function.

edited Jan 16 '18 at 23:52

answered Jan 16 '18 at 19:32

Christophe

54,708
5
52
107

1

Thank you for the response. I had to modify `*that->p_B;` to `*that.p_B;` for the program to compile. I only create one instance of the class A. And inside this class's constructor I instantiate a new instance of the class B to which p_B points to `p_B = new B();`. So how do the copy constuctor and the copy assignment operator functions of class A help me? They are never invoked since I instantiate an object a of class A only once by running `A a;`. – Anton Savelyev Jan 16 '18 at 19:56
Also, after forgot to mention, that after inserting the definitions of the copy constructor and the copy assignment operator into the A class, the program still runs into the _BLOCK_TYPE_IS_INVALID(pHead->nBlockUse) assertion failure. – Anton Savelyev Jan 16 '18 at 20:01
1

Right ! Sorry, for `that`: a kind of reflex coming from `this` ;-). If you really don't need the two missing member functions, declare them as deleted (see edit) to let the compiler avoid the potential mess. But according to your code, the problem doesn't come from A. I'm puzzled thoug: you create A, which allocates an object, but then you never use it and its destruction causes a problem causes a problem when you delete the object ? Then either the pointer was deleted somewhere else (after all it's public) or you have some memory corruption in the resto of the code. – Christophe Jan 17 '18 at 00:07
Right, I only declare it and once the function call completes in which the object is declared, it should be destroyed, and then the issue comes up. The assertion error pops up when I include _DEBUG in my preprocessor definitions field of project config. If I instead exclude that keyword (or use NDEBUG) then everything works fine. I don't do anything else to the A object at all. I'll put a link to the project in the original post. – Anton Savelyev Jan 17 '18 at 00:27

score 1 · Accepted Answer · answered Jan 17 '18 at 19:39

I'm certain the issue doesn't stem from the code but rather from the project configuration properties. It seems that this error comes up when /MD is paired with _DEBUG. However when a project is taken into Release Build, _DEBUG shouldn't be defined, but rather NDEBUG should be. Once I change the pre-processor definition from _DEBUG to NDEBUG, the assertion failure doesn't come up anymore.

It is worth noting that the project executes without issue when using /MDd and _DEBUG.

Deletion of a class pointer in release build causes memory issues

2 Answers2