CODEWITHSUNDEEP
c++delete-operator
463035818_is_not_an_ai
463035818_is_not_an_ai

Reputation: 122984

How to prevent double delete without smart pointers?

Maybe this is a stupid question, but I just want to make sure not to mess up stuff. Suppose I have this struct:

struct Foo{
    struct Bar { virtual int calc(int x) = 0; };
    Bar* barX;
    Bar* barY;
    int x,y;
    Foo(Bar* bx,Bar* by) : barX(by),barY(by) {}
    void process(int xx,int yy){
        x = barX->calc(xx);
        y = barY->calc(yy);
    }
    ~Foo();
};

struct IDBar : Foo::Bar { int calc(int x) { return x; } };

int main() {
    IDBar* b = new IDBar();
    Foo f = Foo(b,b);
    Foo f2 = Foo(new IDBar(),new IDBar());
}

I cannot use C++11 (ie no smartpointers), and I am not 100% sure... Is this the correct way to delete both (or possibly only one) of the Bar objects:

Foo::~Foo(){
    if (barX == barY){ delete barX; }
    else { delete barX; delete barY; }
}

?

PS: The idea is that Foo owns the Bar objects (thus would be responsible for deleting them). The Bars passed to the constructor are not supposed to be used for anything else. Actually a Bar can only belong to a single Foo (a flaw I realized only later, but that is fine for now). Moreover, Foo is not supposed to be copied (maybe I should prevent that explicitly).

Upvotes: 3

Views: 2419

Answers (4)

Jordan M
Jordan M

Reputation: 1243

I have a slight disagreement with Taztingo. It's not unreasonable for an object to take ownership of a resource passed into it. This occurs in all sorts of real world code. The fact that the class takes ownership of resources passed into the constructor is documented and that's that. If someone improperly uses the class without consulting the documentation, they're shooting themselves in the foot.

It is true, however, that this is error prone, and we would like to catch these sorts of errors or prevent them from happening. If there was some way to guarantee that nobody else thinks they own the resource when Foo does, we'd be on our way. In C++11, this is easy with std::unique_ptr. In earlier C++ standards, there is std::auto_ptr. While earlier standards of C++ don't have move constructors, std::auto_ptr functions similarly by relinquishing the resource from an old std::auto_ptr upon assignment or copying to a new std::auto_ptr.

I recommend you use std::auto_ptr.

If the situation where the same resource is explicitly allowed to be passed in for both constructor parameters for the class Foo, create a new constructor that accepts only one std::auto_ptr<Bar>.

Finally, if you want to make it so that Foo cannot be copied, simply declare the copy constructor and assignment operator both as private.

Upvotes: 2

Roland
Roland

Reputation: 336

If you insist in your construction, and cannot heed the design rule of Taztingo; then the answer is yes: it is the correct way!

And then make a strong API documentation of the class Foo and it’s constructor, that the lifetime responsibility of the objects Bar is taken over.

Upvotes: 1

hauron
hauron

Reputation: 4668

I'd argue the comparisons are far from perfect - just imagine you had three pointers.

To ensure double delete never occurs, how about you call delete only one? Consider the scenario:

  1. Allocate a pool of memory (let's say enought to allocate a thousand objects). This should be allocated on the heap via a regular new call, somewhere inside a new PoolAllocator<T> class, and delete within its desctructor.
  2. Implement the pool so it supports the function void* getMemory<T>() returning memory for T to use, and advancing internal pointer.
  3. Make sure whatever the T is called can only be created via placement new on the pool provided memory.

In the end, you need not worry about the memory - the pool object can be a regular stack variable, with internals on the heap.

Otherwise, what forbids you implementing your own proto-smart pointers?

Upvotes: 2

Taztingo
Taztingo

Reputation: 1945

no no no no. god no 

Foo::~Foo(){
        if (barX == barY){ delete barX; }
        else { delete barX; delete barY; }
    }

It is very dangerous to delete memory that the object did not allocate by its self. Whatever owns those objects should delete them. In your case main should be in charge of deleting those allocated objects.

int main() {
    IDBar* b = new IDBar();
    IDBar* b2 = new IDBar();
    IDBar* b3 = new IDBar();
    Foo f = Foo(b,b);
    Foo f2 = Foo(b2,b3);
    delete b;
    delete b2;
    delete b3;
}

A perfect example of an object handling memory management in itself is a LinkedList. A LinkedList creates nodes inside its self. The user has no knowledge of them, and instead just inserts T. The LinkedList will then delete them when it is deleted. The user is only in charge of deleting the LinkedList because he or she created that.

std::list<MyObject*> list;
MyObject* object = new MyObject;
list.push_back(object);
//The list isn't going to delete object because it doesn't own it
delete object;

Upvotes: 1

Related Questions