I've been writing a test case program to demonstrate a problem with a larger program of mine, and the test case has a bug that the original program does not.
Here's the header file:
// compiled with g++ -I/usr/local/bin/boost_1_43_0 -Wall -std=c++0x -g test.cpp
#include <bitset>
#include <boost/shared_ptr.hpp>
#include <vector>
typedef std::vector< std::vector< std::bitset<11> > > FlagsVector;
namespace yarl
{
namespace path
{
class Pathfinder;
}
namespace level
{
class LevelMap
{
// Member Variables
private:
int width, height;
FlagsVector flags;
public:
boost::shared_ptr<path::Pathfinder> pathfinder;
// Member Functions
LevelMap(const int, const int);
int getWidth() const {return width;}
int getHeight() const {return height;}
bool getFifthBit(const int x, const int y) const
{
return flags.at(x).at(y).test(5);
}
};
class Level
{
// Member Variables
public:
LevelMap map;
// Member Functions
public:
Level(const int w=50, const int h=50);
};
}
namespace path
{
class Pathfinder
{
// Member Variables
private:
boost::shared_ptr<level::LevelMap> clientMap;
// Member Functions
public:
Pathfinder() {}
Pathfinder(level::LevelMap* cm)
: clientMap(cm) {}
void test() const;
};
}
}
and here's the implementation file:
#include <iostream>
#include "test.hpp"
using namespace std;
namespace yarl
{
namespace level
{
LevelMap::LevelMap(const int w, const int h)
: width(w), height(h), flags(w, vector< bitset<11> >(h, bitset<11>())),
pathfinder(new path::Pathfinder(this))
{}
Level::Level(const int w, const int h)
: map(w,h)
{
map.pathfinder->test();
}
}
namespace path
{
void Pathfinder::test() const
{
int width = clientMap->getWidth();
int height = clientMap->getHeight();
cerr << endl;
cerr << "clientMap->width: " << width << endl;
cerr << "clientMap->height: " << height << endl;
cerr << endl;
for(int x=0; x<width; ++x)
{
for(int y=0; y<height; ++y)
{
cerr << clientMap->getFifthBit(x,y);
}
cerr << "***" << endl; // marker for the end of a line in the output
}
}
}
}
int main()
{
yarl::level::Level l;
l.map.pathfinder->test();
}
I link this program with electric fence, and when I run it it aborts with this error:
ElectricFence Aborting: free(bffff434): address not from malloc().
Program received signal SIGILL, Illegal instruction.
0x0012d422 in __kernel_vsyscall ()
backtracing from gdb shows that the illegal instruction is in the compiler-generated destructor of Pathfinder
, which is having trouble destructing its shared_ptr. Anyone see why that is?
yarl::level::Level l;
You instantiate an automatic Level
variable, which, in its constructor constructs its member pathfinder
like so:
pathfinder(new path::Pathfinder(this))
Then in the Pathfinder
constructor, it takes the Level
pointer that you pass in and assigns that to a shared_ptr
. The shared_ptr
then takes ownership of this pointer.
This is incorrect for several reasons:
shared_ptr
should be used to manage dynamically allocated objects, not automatically allocated objectsshared_ptr
, then you should use it everywhere: as it is now, you pass raw pointers (e.g. to the constructor of Pathfinder
, but then store them as shared_ptr
s. This just opens a big can of ownership worms.this
to a shared_ptr
is to derive from enable_shared_from_this
; note however that you cannot get a shared_ptr
from this
in a constructor.When the shared_ptr
is destroyed, it will try to delete the pointer it manages. In this case, however, that pointer is not to a dynamically allocated object (i.e., allocated with new
), but to an automatically allocated object (i.e., on the stack). Hence, the error.
If you don't need something to take ownership of a resource, there is nothing wrong with using a raw pointer (or a reference, if you have that option).