Visual Studio 2010 C++ runtime error

Question

I came across strange behavior in Visual Studio 2010 C++ compiler. Following code compiles but throws "Debug assertion failed" after execution with message:

"_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)"

Compiles and runs smoothly under GCC. Is it my fault?

#include <iostream>
#include <vector>


using namespace std;

typedef unsigned int uint;


class Foo {
    vector<int*> coll;
public:

    void add(int* item) {
       coll.push_back(item);
    }

    ~Foo() {
        for (uint i = 0; i < coll.size(); ++i) {
            delete coll[i];
            coll[i] = NULL;
        }
    }
};

int main()
{
   Foo foo;
   foo.add(new int(4));
   Foo bar = foo;

   return 0;
}

That error is thrown by the VS debug runtime; if you'd compiled it release then you wouldn't get the error but you do risk a crash. If by GCC you meant GCC on Linux then you can see the same error by running your code under valgrind. — Rup, May 06 '11 at 15:57

score 7 · Accepted Answer · answered May 06 '11 at 15:54

You didn't implement a copy constructor and copy assignment operator (see rule of three). This results in a shallow copy of the pointers in your vector, causing a double delete and the assertion. EDIT: Double delete is undefined behavior so both VS and gcc are correct here, they're allowed to do whatever they want.

Typically when you implement a destructor with non-trivial behavior you'll also need to write or disable copy construction and copy assignment.

However in your case do you really need to store the items by pointer? If not, just store them by value and that would fix the problem. Otherwise if you do need pointers use shared_ptr (from your compiler or boost) instead of raw pointers to save you from needing to write your own destructor/copy methods.

EDIT: A further note about your interface: Interfaces like this that transfer ownership of passed in pointers can cause confusion by people using your class. If someone passed in the address of an int not allocated on the heap then your destructor will still fail. Better is to either accept by value if possible, or clone the passed in item making your own call to new in the add function.

I would not blindy recommend `shared_ptr`, their behavior is complicated (strong / weak references) and thus they are better avoided when other solutions can be used. Here I can see `std::vector`, `std::vector< std::unique_ptr >` and `boost::ptr_vector` as superior contenders. — Matthieu M., May 06 '11 at 16:30

score 3 · Answer 2 · answered May 06 '11 at 15:54

3

You're deleting item twice, because the line

Foo bar = foo;

Invokes the default copy constructor, which duplicates the itempointer, rather than allocating and copying the data.

answered May 06 '11 at 15:54

Paul Beckingham

14,495
5
33
67

score 2 · Answer 3 · edited May 23 '17 at 12:21

The problem is both the bar and foo member's vector element is same. When foo goes out of scope it's destructor is called which deallocates the pointer leaving the bar vector element dangling.bar destructor tries to deallocate it's vector element which was left dangling and is causing you the runtime error. You should write a copy constructor.

Foo bar = foo; // Invokes default copy constructor.

Edit 1: Look at this thread to know about Rule of three

score 0 · Answer 4 · answered May 06 '11 at 16:34

The simpler solution here is not to use int* in the first place.

#include <iostream>
#include <vector>


using namespace std;

typedef unsigned int uint;


class Foo {
    vector<int> coll; // remove *
public:

    void add(int item) { // remove *
       coll.push_back(item);
    }

    // remove ~Foo
};

int main()
{
   Foo foo;
   foo.add(4); // remove `new` call
   Foo bar = foo;

   return 0;
}

In general, try to avoid new.

If you cannot, use a smart manager (like std::unique_ptr) to handle the memory clean-up for you.

In any case, if you're calling delete manually, you're doing it wrong. Note: not calling delete and letting the memory leak is wrong too

Visual Studio 2010 C++ runtime error

4 Answers4