Problem deleting object which contain std::tr1::shared_ptr

Started by
7 comments, last by rip-off 14 years, 3 months ago
Hi, I get the dbgdel.cpp line: 52 Debug Assertion Failed when I delete an object which contains a shared_ptr (std::tr1, not boost). I've tried deleting just the shared_ptr from the object and then it works perfectly. This is the object .h file: ---------------------------------------------------- #ifndef BOMB_H #define BOMB_H #include <stdlib.h> #include <memory> #include <vector> #include <algorithm> #include <functional> #include "DiceInvaders.h" using namespace std::tr1; class Bomb { public: Bomb(ISprite* sprite, int gameHeight); ~Bomb(); void move(float amount); bool isEnabled() { return enabled; } void setEnabled(bool enabled, float xPos, float yPos); void draw(); float getX() { return xPos; } float getY() { return yPos; } private: float speed; float xPos; float yPos; bool enabled; int gameHeight; shared_ptr<ISprite> sprite; }; #endif ---------------------------------------------------- And this is the .cpp file: ---------------------------------------------------- #include "Bomb.h" Bomb::Bomb(ISprite* sprite, int gameHeight) { this->sprite.reset(sprite); speed = 200.0f; enabled = false; this->gameHeight = gameHeight; } Bomb::~Bomb() { } void Bomb::move(float amount) { if(enabled) { yPos += amount * speed; if(yPos+32 > gameHeight) enabled = false; } } void Bomb::setEnabled(bool enabled, float xPos, float yPos) { this->enabled = enabled; this->xPos = xPos; this->yPos = yPos; } void Bomb::draw() { if(enabled) { sprite->draw((int)xPos, (int)yPos); } } ----------------------------------------------------- The following code gets the error if I put it in the main file: ----------------------------------------------------- Bomb* b = new Bomb(bomb, 1); //bomb is an ISprite delete b; ----------------------------------------------------- I don't know why this happends except that it doesn't happen if I don't have a shared_ptr in the class. Any help is welcome, thanks! [Edited by - startselect on January 9, 2010 7:58:19 AM]
Advertisement
Does ISprite have a virtual destructor?
It has a virtual void destroy() but thats not the same right?

struct ISprite
{
// Destroys the sprite instance
virtual void destroy() = 0;

// Draw the sprite at the given position.
// Valid coordinates are between (0,0) (upper left) and (width-32, height-32) (lower right).
// (All sprites are 32*32 pixels, clipping is not supported)
virtual void draw(int x, int y) = 0;
};

This is all I know of it, the real sprite class is in a .dll which I can't view.
Quote:Original post by startselect
Bomb* b = new Bomb(bomb, 1); //bomb is an ISprite
delete b;

Where does 'bomb' come from? In general you can only use shared_ptr for instances where you control the life time yourself, since shared_ptr will delete the instance when the last shared_ptr goes out of scope. For example (contrieved example...)
int * foo = new int();{    shared_ptr ptr( foo );}delete foo; // Whoops, foo was already deleted!

In your example, if bomb is an instance you get from across a DLL boundary, then you aren't allowed to delete it, since delete must be called in the same DLL that new'ed. So you can't store bomb in a shared_ptr.
Thank you so much, switching to normal pointers fixed everything!
It turns out I should have never used shared_ptr, it just seemed correct when I thought about alot of bombs having the same sprite.

Thanks again!
No.
Using shared_ptr<>s correctly will be much better than using raw pointers all over the place.

The only reason that you're not crashing if you have a normal pointer in the struct is that you never actually release the ISprite instance in that case. This is not a shortcoming of shared_ptr<>.

[size=1]Visit my website, rawrrawr.com

Your Bomb constructor should take a shared_ptr<ISprite> as an argument. With your original code, if you passed the same raw pointer to mulitple Bomb instances, you would have issues. Remember, when you pass a raw pointer to the constructor (or reset function) of a shared_ptr<>, it means you are passing ownership of that pointer to the shared_ptr<>. Ideally, this is the last time you will use the raw pointer to that resource.

Your ISprite class should feature a virtual destructor. A "destroy" method is typically not required in C++ and will often create bugs where you forget to destroy an instance, or you access an instance that hasn't been destructed but has been destroyed.
I can't change the game library I'm using and it's createSprite function only gives me a raw ISprite pointer. The only way to free that memory seems to be to use the destroy method presented in the ISprite struct. So if I create a shared_ptr<ISprite> out of that raw pointer it will try to delete something it didn't create and thats why I got the error, right? If I could I would but I can't so I won't, but I understand how it would be much better with a virtual destructor in ISprite that the shared_ptr could call.

I am however using raw pointers now, and the game still works like it should and vld reports no memory leaks.
Ok, I thought the sprite class was your own.

You can do something like this:
struct DeleteSprite {    void operator()(ISprite *sprite) {        sprite->destroy();    }};std::tr1::shared_ptr<ISprite> createSpriteWrapper(/* ... args ... */){   std::tr1::shared_ptr<ISprite> sprite(createSprite(/* .. args .. */, DeleteSprite());   return sprite;}

Raw pointers can be managed, but you end up having to write lots of code to ensure they are handled correctly. For example, does your Bomb class follow the rule of three? If not, you could risk memory leaks or undefined behaviour.

Also, by switching to raw pointers you make it harder to share sprites between Bomb instances, which result in unnecessary memory overhead for your game objects.

This topic is closed to new replies.

Advertisement