I'm not sure if it's a bug or not, but when I use string-type members inside structures or classes, valgrind identifies memory leaks. I've tried to build a simple code based on my own application, I'm sorry if it is still big...
// ====================================================================
#include <iostream>
#include <string>
#include <sstream>
#include <vector>
using namespace std;
// ====================================================================
string int2str(const int &i) {
return static_cast<ostringstream*>(
&(ostringstream() << i))->str();
}
// ====================================================================
class P;
// ====================================================================
struct Node {
virtual char isType() const = 0;
};
// ====================================================================
struct X : Node {
string st;
int id;
X(const string &_st, const int &_id);
char isType() const { return 'x'; };
// Those member functions are after class P declaration:
P use_as_P();
P use_as_P(const P &arg0);
P use_as_P(const P &arg0, const P &arg1);
};
X::X(const string &_st, const int &_id) : st(_st), id(_id) { }
// ====================================================================
class P {
friend struct X;
private:
Node *node;
vector<P> children;
public:
P() : node(NULL) {};
P(const P &source);
void swap(P &other);
string print_this();
~P();
};
P::P(const P &source) {
this->children = source.children;
switch(source.node->isType()) {
case 'x':
this->node = new X(static_cast<X*>(source.node)->st,
static_cast<X*>(source.node)->id);
break;
}
}
void P::swap(P &other) {
std::swap(this->node, other.node);
std::swap(this->children, other.children);
}
string P::print_this() {
string msg = "( ";
msg += static_cast<X*>(this->node)->st;
msg += int2str(static_cast<X*>(this->node)->id);
msg += " ";
for(size_t i = 0; i < this->children.size(); i++)
msg += children.at(i).print_this();
msg += ") ";
return msg;
}
P::~P() {
if(this->node != NULL)
delete node;
this->children.clear();
}
// ====================================================================
P X::use_as_P() {
P ast_aux;
ast_aux.node = new X(this->st,this->id);
return ast_aux;
}
P X::use_as_P(const P &arg0) {
P ast_aux;
ast_aux.node = new X(this->st,this->id);
ast_aux.children.push_back(arg0);
return ast_aux;
}
P X::use_as_P(const P &arg0, const P &arg1) {
P ast_aux;
ast_aux.node = new X(this->st,this->id);
ast_aux.children.push_back(arg0);
ast_aux.children.push_back(arg1);
return ast_aux;
}
// ====================================================================
// ** MAIN **
// ====================================================================
int main(int argc, char **argv)
{
X a("how",0), b("what",1), c("why",2), d("when",3);
P testing = a.use_as_P(b.use_as_P(c.use_as_P()),d.use_as_P());
cout << testing.print_this() << endl;
return 0;
}
// ====================================================================
Compiling with:
nvcc -arch sm_20 -o LEAK_test_with_string LEAK_test_with_string.cu
And here goes the valgrind's analysis:
==5877== Memcheck, a memory error detector
==5877== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==5877== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==5877== Command: ./LEAK_test_with_string
==5877==
( how0 ( what1 ( why2 ) ) ( when3 ) )
==5877==
==5877== HEAP SUMMARY:
==5877== in use at exit: 114 bytes in 4 blocks
==5877== total heap usage: 47 allocs, 43 frees, 3,701 bytes allocated
==5877==
==5877== 28 bytes in 1 blocks are definitely lost in loss record 1 of 4
==5877== at 0x4C2A879: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5877== by 0x5516F38: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x5518640: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x5518A57: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x403691: main (in /home/igor/projects/system_modeling/LEAK_test_with_string)
==5877== : st(_st), id(_id) {}
==5877== 28 bytes in 1 blocks are definitely lost in loss record 2 of 4
==5877== at 0x4C2A879: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5877== by 0x5516F38: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x5518640: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x5518A57: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x40375A: main (in /home/igor/projects/system_modeling/LEAK_test_with_string)
==5877==
==5877== 29 bytes in 1 blocks are definitely lost in loss record 3 of 4
==5877== at 0x4C2A879: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5877== by 0x5516F38: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x5518640: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x5518A57: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x4036F7: main (in /home/igor/projects/system_modeling/LEAK_test_with_string)
==5877==
==5877== 29 bytes in 1 blocks are definitely lost in loss record 4 of 4
==5877== at 0x4C2A879: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5877== by 0x5516F38: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x5518640: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x5518A57: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877== by 0x4037BD: main (in /home/igor/projects/system_modeling/LEAK_test_with_string)
==5877==
==5877== LEAK SUMMARY:
==5877== definitely lost: 114 bytes in 4 blocks
==5877== indirectly lost: 0 bytes in 0 blocks
==5877== possibly lost: 0 bytes in 0 blocks
==5877== still reachable: 0 bytes in 0 blocks
==5877== suppressed: 0 bytes in 0 blocks
==5877==
==5877== For counts of detected and suppressed errors, rerun with: -v
==5877== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 2 from 2)
The "it's a bug" wondering comes when I change the C++ string member into a plain C char* member. One just need to change this part of the above written code:
// ====================================================================
struct X : Node {
char st[6]; // <=============== HERE!
int id;
X(const string &_st, const int &_id);
char isType() const { return 'x'; };
// Those member functions are after class P declaration:
P use_as_P();
P use_as_P(const P &arg0);
P use_as_P(const P &arg0, const P &arg1);
};
X::X(const string &_st, const int &_id) : id(_id) { // <=============== HERE!
strcpy(st, _st.c_str()); // <=============== HERE!
}
// ====================================================================
Note that I still use string in my code, but now there isn't any string member declared. This way, valgrind doesn't complain anymore:
==5977== Memcheck, a memory error detector
==5977== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==5977== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==5977== Command: ./LEAK_test_without_string
==5977==
( how0 ( what1 ( why2 ) ) ( when3 ) )
==5977==
==5977== HEAP SUMMARY:
==5977== in use at exit: 0 bytes in 0 blocks
==5977== total heap usage: 57 allocs, 57 frees, 3,986 bytes allocated
==5977==
==5977== All heap blocks were freed -- no leaks are possible
==5977==
==5977== For counts of detected and suppressed errors, rerun with: -v
==5977== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
Does anybody have an insight on this? I mean, is this a bug that should be reported or is there something that I'm actually missing in my string-version code?
Since no one's bitten the bullet I'll expand my comment: Your Node
class does not have a virtual destructor, this makes the following line
delete node;
invoke undefined behaviour - this will only call the destructor for Node
, the destructor for X
is never called.
The solution is simple, provide a virtual destructor for Node
:
struct Node {
virtual char isType() const = 0;
virtual ~Node() =default;
// or virtual ~Node(){} if your compiler does not support defaulted functions.
}
You can now safely delete
base pointers of Node
that point to inherited classes.
The reason for the memory leak when using a std::string
is because when you delete the base pointer node
, the destructor for X
is not called, and so neither is the destructor for st
. The char
array version doesn't leak because node
points to a simple block of memory, and X
in this case doesn't have any complex members that require destruction, so delete node
luckily releases all of the memory allocated for X
and its members. Remember that this is undefined behaviour however and the compiler can legitimately do whatever it wants to.
If you have a base class with even a single virtual
function, always add a virtual destructor. You can make a case for omitting a virtual destructor if you are absolutely sure you won't be deleting inherited classes using a pointer to the base class, but it's safer to just add one anyway just in case (the gcc compiler flag -Weffc++
will also tell you as much).