How to safely delete multiple pointers
Asked Answered
C

8

5

I have got some code which uses a lot of pointers pointing to the same address. Given a equivalent simple example:

int *p =  new int(1);
int *q = p;
int *r = q;

delete r; r = NULL; // ok
// delete q; q = NULL; // NOT ok
// delete p; p = NULL; // NOT ok

How to safely delete it without multiple delete? This is especially difficult if I have a lot of objects which having pointers all pointing to the same address.

Cyanocobalamin answered 14/10, 2010 at 10:53 Comment(5)
Shouldn't this work? delete null is specified in the standard, so it allowed and should work. OK, it is not best coding style...Englebert
@Mario: Deleting NULL is specifed as a NO-OP but calling it does incur some overhead.Eolith
Problem is q and p won't be NULL, so there will be double deletes.Clone
@Mario You're not calling delete on NULL when you delete q and p; you're calling delete on the old address of that integer, which q and p are still holding on to.Addicted
ooops, shouldn't post shortly afeter lunch... thanx for correcting me!Englebert
A
18

The answer, without resorting to managed pointers, is that you should know whether or not to delete a pointer based on where it was allocated.

Your example is kind of contrived, but in a real world application, the object responsible for allocating memory would be responsible for destroying it. Methods and functions which receive already initialized pointers and store them for a time do not delete those pointers; that responsibility lies with whatever object originally allocated the memory.

Just remember that your calls to new should be balanced by your calls to delete. Every time you allocate memory, you know you have to write balancing code (often a destructor) to deallocate that memory.

Addicted answered 14/10, 2010 at 10:58 Comment(3)
+1: My opinion is that shared_ptr should only be used in corner case where there is no clear owner of the memory and that in most case you don't need it.Assemble
Please read: crazyeddiecpp.blogspot.com/2010/12/pet-peeve.html - you do not delete pointers!Inalienable
@Noah Thanks, but no thanks. I will continue to say "delete the pointer" instead of "free the memory that the pointer is pointing to".Addicted
A
28

Your tool is shared_ptr of the boost library. Take a look at the documentation: http://www.boost.org/doc/libs/1_44_0/libs/smart_ptr/shared_ptr.htm

Example:

void func() {
  boost::shared_ptr<int> p(new int(10));
  boost::shared_ptr<int> q(p);
  boost::shared_ptr<int> r(q);

  // will be destructed correctly when they go out of scope.
}
Addams answered 14/10, 2010 at 10:55 Comment(7)
+1 - OP's scenario of 'a lot of pointers pointing to the same address' is ideal for shared_ptrHelaine
Recently released compilers provide shared_ptr as a member of the tr1 namespace. So boost is not required -- you can use tr1::shared_ptr instead.Federate
"Raw" pointers in C++ are inherited from C, but the problem is that they don't express ownership at all (it is one of the issues that can't be mentioned frequently enough, IMHO). One of the most important things about code is that it must express the author's intentions (including ownership), so it's always better to use the boost smart pointers, or at least std::auto_ptr. (But old-school pointers are fine for simple cases, like internals for 'trivially-behaving' objects).Alainaalaine
@riviera: +1, the most important word that seems to be forgotten from most pointer discussions is ownership.Coventry
@riviera: Another great tool to express ownership is the comment. One of its virtues is that it doesn't bring any performance or space penalty.Clone
@kotlinski: Although I use comments, too, I prefer to the code to express its intentions itself. An obvious advantage is that once the compiler is 'aware' of what you want to do, it can apply its type safety checkings, etc. (not mentioning the saying "you don't write the code for the computer, you write it for other human beings"). But I don't want to get too academic, and I already said what find most important in my previous comment :)Alainaalaine
Smart pointers are a great tool and should certainly be used. But on their own they are not sufficient to "solve" this problem. Cycles of shared_ptrs will result in leaks. You still need to keep track of ownership and use weak_ptr for pointers that might outlive the owner's handle.Embryology
A
18

The answer, without resorting to managed pointers, is that you should know whether or not to delete a pointer based on where it was allocated.

Your example is kind of contrived, but in a real world application, the object responsible for allocating memory would be responsible for destroying it. Methods and functions which receive already initialized pointers and store them for a time do not delete those pointers; that responsibility lies with whatever object originally allocated the memory.

Just remember that your calls to new should be balanced by your calls to delete. Every time you allocate memory, you know you have to write balancing code (often a destructor) to deallocate that memory.

Addicted answered 14/10, 2010 at 10:58 Comment(3)
+1: My opinion is that shared_ptr should only be used in corner case where there is no clear owner of the memory and that in most case you don't need it.Assemble
Please read: crazyeddiecpp.blogspot.com/2010/12/pet-peeve.html - you do not delete pointers!Inalienable
@Noah Thanks, but no thanks. I will continue to say "delete the pointer" instead of "free the memory that the pointer is pointing to".Addicted
C
10

The "modern" answer is to use a smart pointer and don't do any manual deletes.

boost::shared_ptr<int> p(new int(1));
boost::shared_ptr<int> q = p;
boost::shared_ptr<int> r = q;

End of story!

Clone answered 14/10, 2010 at 10:56 Comment(1)
-1 for posting untested code. A good smart pointer API (such as of std::auto_ptr of boost::shared_ptr) souldn't allow to assign pointers to smart pointers or create smart pointers implicit from pointers using copy constructor. this way the ownership wouldn't be obvious. Change your first line to boost::shared_ptr<int> p = boost::shared_ptr<int>(new int(1));Tutorial
A
3

The problem you are facing is that the ownership semantics in your program are not clear. From a design point of view, try to determine who is the owner of the objects at each step. In many cases that will imply that whoever creates the object will have to delete it later on, but in other cases ownership can be transferred or even shared.

Once you know who owns the memory, then go back to code and implement it. If an object is the sole responsible for a different object, the it should hold it through a single-ownership smart pointer (std::auto_ptr/unique_ptr) or even a raw pointer (try to avoid this as it is a common source of errors) and manage the memory manually. Then pass references or pointers to other objects. When ownership is transfered use the smart pointer facilities to yield the object to the new owner. If the ownership is truly shared (there is no clear owner of the allocated object) then you can use shared_ptr and let the smart pointer deal with the memory management).

Afore answered 14/10, 2010 at 11:44 Comment(0)
T
1

Why are you trying to delete pointers arbitrarily? Every dynamically allocated object is allocated in one place, by one owner. And it should be that one owners responsibility to ensure the object is deleted again.

In some cases, you may want to transfer ownership to another object or component, in which case the responsibility for deleting also changes.

And sometimes, you just want to forget about ownership and use shared ownership: everyone who uses the object shares ownersip, and as long as at least one user exists, the object should not be deleted.

Then you use shared_ptr.

In short, use RAII. Don't try to manually delete objects.

Tourneur answered 14/10, 2010 at 11:47 Comment(0)
R
0

There are some very rare cases where you might not be able to use a smart pointer (probably dealing with old code), but cannot use a simple "ownership" scheme either.

Imagine you have an std::vector<whatever*> and some of the whatever* pointers point to the same object. Safe cleanup involves ensuring you don't delete the same whatever twice - so build an std::set<whatever*> as you go, and only delete the pointers that aren't already in the set. Once all the pointed-to objects have been deleted, both containers can safely be deleted as well.

The return value from insert can be used to determine whether the inserted item was new or not. I haven't tested the following (or used std::set for a while) but I think the following is right...

if (myset.insert (pointervalue).second)
{
  //  Value was successfully inserted as a new item
  delete pointervalue;
}

You shouldn't design projects so that this is necessary, of course, but it's not too hard to deal with the situation if you can't avoid it.

Revamp answered 14/10, 2010 at 11:29 Comment(0)
B
0

It is not possible to know whether the memory referenced by a pointer has already been deleted, as in your case.
If you can not use a library with smart pointers, that do reference counting, and you can not implement your own reference counting schema (if indeed you need to do what you describe in your post) try to call realloc on the pointers.
I have read in other posts that depending on the implementation the call to realloc may not crash but return back a null pointer. In this case you know that the memory referenced by that pointer has been freed.
If this works as a dirty solution, it will not be portable, but if you have no other option, try it. The worse of course will be to crash your application :)

Buhrstone answered 14/10, 2010 at 13:13 Comment(0)
P
0

I would say that some times, smart pointers can actually slow down your application, albiet not by a lot. What I would do is create a method, like so:

void safeDelete(void **ptr)
{
  if(*ptr != NULL)
  {
     delete ptr;
     *ptr = NULL;
  }
}

I'm not sure if I did it 100% correctly, but what you do is you pass the pointer into this method that takes the pointer of a pointer, checks to make sure the pointer it points to isn't set to NULL, then deletes the object and then sets the address to 0, or NULL. Correct me if this isn't a good way of doing this, I'm also new to this and someone told me this was a great way of checking without getting complicated.

Phonolite answered 24/6, 2012 at 18:5 Comment(1)
This does not solve the problem; with the example in the question, safeDelete(&r) will work, but it won't modify q or p, so safeDelete(&q) will be likely to crash.Kaliski

© 2022 - 2024 — McMap. All rights reserved.