I need to add objects of the same class to a vector:
#include <vector>
#include <cstdio>
class A {
int *array;
int size;
public:
A(int s) {
array = new int[size = s];
fprintf(stderr, "Allocated %p\n", (void*)array);
}
~A() {
fprintf(stderr, "Deleting %p\n", (void*)array);
delete array;
}
};
int main() {
std::vector<A> v;
for (int n = 0; n < 10; n++) {
fprintf(stderr, "Adding object %d\n", n);
v.push_back(A(10 * n));
//v.emplace_back(10 * n);
}
return 0;
}
When I run this program, it crashes after producing the following output:
Adding object 0
Allocated 0x146f010
Deleting 0x146f010
Adding object 1
Allocated 0x146f050
Deleting 0x146f010
*** glibc detected *** ./a.out: double free or corruption (fasttop): 0x000000000146f010 ***
It seems that the destructor of the 0th object is called when adding the 1st object. Even stranger is when I use emplace_back instead of push_back:
Adding object 0
Allocated 0x1644030
Adding object 1
Allocated 0x1644080
Deleting 0x1644030
Adding object 2
Allocated 0x1644100
Deleting 0x1644030
Deleting 0x1644080
Adding object 3
Allocated 0x1644160
Adding object 4
Allocated 0x1644270
Deleting 0x1644030
*** glibc detected *** ./a.out: double free or corruption (fasttop): 0x0000000001644030 ***
Can someone explain why this is happening, and the correct way to do this? Compiler used was g++ 4.7.2 under Linux, but I'm also getting the same behavior with clang 7.3.0 under Mac OS X.
Your A class does not follow the Rule of Three:
The rule of three (also known as the Law of The Big Three or The Big Three) is a rule of thumb in C++ (prior to C++11) that claims that if a class defines one (or more) of the following it should probably explicitly define all three:
- destructor
- copy constructor
- copy assignment operator
These three functions are special member functions. If one of these functions is used without first being declared by the programmer it will be implicitly implemented by the compiler with the default semantics of performing the said operation on all the members of the class.
- Destructor – Call the destructors of all the object's class-type members
- Copy constructor – Construct all the object's members from the corresponding members of the copy constructor's argument, calling the copy constructors of the object's class-type members, and doing a plain assignment of all non-class type (e.g., int or pointer) data members
- Copy assignment operator – Assign all the object's members from the corresponding members of the assignment operator's argument, calling the copy assignment operators of the object's class-type members, and doing a plain assignment of all non-class type (e.g. int or pointer) data members.
The Rule of Three claims that if one of these had to be defined by the programmer, it means that the compiler-generated version does not fit the needs of the class in one case and it will probably not fit in the other cases either. The term "Rule of three" was coined by Marshall Cline in 1991.
An amendment to this rule is that if the class is designed in such a way that Resource Acquisition Is Initialization (RAII) is used for all its (nontrivial) members, the destructor may be left undefined (also known as The Law of The Big Two). A ready-to-go example of this approach is the use of smart pointers instead of plain ones.
Because implicitly-generated constructors and assignment operators simply copy all class data members ("shallow copy"), one should define explicit copy constructors and copy assignment operators for classes that encapsulate complex data structures or have external references such as pointers, if you need to copy the objects pointed to by the class members. If the default behavior ("shallow copy") is actually the intended one, then an explicit definition, although redundant, will be a "self-documenting code" indicating that it was an intention rather than an oversight.
You need to add a copy constructor and a copy assignment operator (and your destructor needs to use delete[] instead of delete):
class A
{
private:
int *array;
int size;
public:
A(int s)
: size(s), array(new int[s])
{
fprintf(stderr, "Allocated %p\n", array);
}
A(const A &src)
: size(src.size), array(new int[src.size])
{
std::copy(src.array, src.array + src.size, array);
fprintf(stderr, "Allocated %p, Copied from %p\n", array, src.array);
}
~A()
{
fprintf(stderr, "Deleting %p\n", array);
delete[] array;
}
A& operator=(const A &rhs)
{
A tmp(rhs);
std::swap(array, tmp.array);
std::swap(size, tmp.size);
return *this;
}
};
Since you mention emplace_back(), that means you are using C++11 or later, which means you should also deal with move semantics of the Rule of Five:
With the advent of C++11 the rule of three can be broadened to the rule of five as C++11 implements move semantics, allowing destination objects to grab (or steal) data from temporary objects. The following example also shows the new moving members: move constructor and move assignment operator. Consequently, for the rule of five we have the following special members:
- destructor
- copy constructor
- move constructor
- copy assignment operator
- move assignment operator
Situations exist where classes may need destructors, but cannot sensibly implement copy and move constructors and copy and move assignment operators. This happens, for example, when the base class does not support these latter Big Four members, but the derived class's constructor allocates memory for its own use.[citation needed] In C++11, this can be simplified by explicitly specifying the five members as default.
You should add a move constructor and a move assignment operator to the above code:
class A
{
private:
int *array;
int size;
public:
A(int s)
: size(s), array(new int[s])
{
fprintf(stderr, "Allocated %p\n", array);
}
A(const A &src)
: size(src.size), array(new int[src.size])
{
std::copy(src.array, src.array + src.size, array);
fprintf(stderr, "Allocated %p, Copied from %p\n", array, src.array);
}
A(A &&src)
: size(0), array(nullptr)
{
std::swap(array, src.array);
std::swap(size, src.size);
fprintf(stderr, "Moved %p, Replaced with %p\n", array, src.array);
}
~A()
{
fprintf(stderr, "Deleting %p\n", array);
delete[] array;
}
A& operator=(const A &rhs)
{
A tmp(rhs);
std::swap(array, tmp.array);
std::swap(size, tmp.size);
return *this;
}
A& operator=(A &&rhs)
{
std::swap(array, rhs.array);
std::swap(size, rhs.size);
return *this;
}
};
Otherwise, you should strive for the Rule of Zero instead:
There's a proposal by R. Martinho Fernandes to simplify all of the above into a Rule of 0 for C++ (primarily for C++11 & newer). The rule of 0 states that if you specify any of the default members, then your class must deal exclusively with a single resource. Furthermore, it must define all default members to handle that resource (or delete the default member as appropriate). Thus such classes must follow the rule of 5 described above. A resource can be anything: memory that gets allocated, a file descriptor, database transaction etc.
Any other class must not allocate any resources directly. Furthermore, they must omit the default members (or explicitly assign all of them to default via = default). Any resources should be used indirectly by using the single-resource classes as member/local variables. This lets such classes inherit the default members from the union of member variables, thereby auto-forwarding the movability/copyability of the union of all underlying resource. Since ownership of 1 resource is owned by exactly 1 member variable, exceptions in the constructor cannot leak resources due to RAII. Fully initialized variables will have their destructors called & uninitialized variables could not have owned any resources to begin with.
Since the majority of classes don't deal with ownership as their sole concern, the majority of classes can omit the default members. This is where the rule-of-0 gets its name.
Eliminate your manual array altogether and use a std::vector instead:
class A
{
private:
std::vector<int> array;
public:
A(int s)
: array(s)
{
}
};
No need to explicitly define a copy/move constructor, copy/move assignment operator, or a destructor, because the default implementations provided by the compiler will automatically call the corresponding functionality of the vector for you.
