Freeing a Pointer if memory isn't being referenced by anything else

Question

I have a method that has a few pointers as parameters. This method can be called with either named pointers from the callee or dynamically create a pointer to a new object and pass it in as an argument directly as the method is being called.

myClass *myPtr = new myClass(...);
myMethod(myPtr);

Verus

myMethod(new myClass(...));

The problem is that if both of these are valid options, how does one properly free the passed in pointer? Deleting myPtr within myMethod will cause a crash if myPtr is ever accessed again within the program. If I don't delete myPtr, the second option will cause a memory leak if it is used. There are benefits for using both options so both shouldn't break the program.

Aside from using STL, what are some solutions to this problem? Would I have to implement my own garbage collector?

Answer 1

You can use a smart pointer for that, like shared_ptr from boost.

If not you need to state clearly who owns the object. If you are going to take over the ownership or leave it to the caller.
If you leave it to the caller, using the form function(new whatever()) wouldn't be a good idea, but the leak would be responsibility of the caller.

If you intent to take over the ownership, creating a sink method, choosing a proper name would be a good idea, of course you'd need to delete the objects by yourself once you are finished.

Answer 2

First thing, you need to assign ownership of the object: you need to have a consistent strategy (and enforced!) as to "who" has the right to delete an object. Once this is clearly established, you shouldn't have the problem you are running into.

Some strategies:

1. Object is leased: ownership is retained by lender

2. Object is given: ownership is transferred

Second, for tracking usage of an object, you need an infrastructure such as "smart pointers". Here you have 2 categories to care about:

1. Object is "singly referenced" i.e. only one "user"

2. Object is "multi referenced" i.e. more than one "user" at one point in time

For (1), the "tracking information" is the pointer itself whereas in (2) you need more infrastructure.

Answer 3

I would say, in this case caller should be responsible for freeing the object. You can consider various options, simplest is:

myClass myInstance = myClass;  // or myClass(arg1, arg2, ...)
// and the pass it to your method like this:
myMethod(&myInstance);

You could also consider some smart pointer options like std::tr1::shared_ptr or something from boost.

UPDATE: If your method should be able to get NULL-pointer as its argument, there's no problem at all:

// this is your method declaration:
void myMethod(const myClass *myPtr);

// in your tests or wherever in your code you can call it like
myClass myInstance = myClass;  // or myClass(arg1, arg2, ...)
myMethod(&myInstance);
// or like this:
myMethod(NULL);
// for as long as your method has something like this in it:
if (myPtr)
    myPtr->someMethod();

Answer 4

You should use some sort of smart pointer.
For this simple case ever auto_ptr is fine but in general you should use scoped_ptr or shared_ptr.
If you have some unexplained phobia against STL or boost, you can always pull your own smart pointer class with relative ease (if you don't require super exception safety).

Answer 5

This is a all about ownership.

You need to decide who has ownership (or is there shared ownership).
Basically passing pointers is a very bad idea and NOT very C++ like (this is basically a C interface as there is no concept of ownership). In a C++ program you should define your interfaces (functions) with a very clear sense of ownership transfer.

You have a couple of options.

1. The function does not have ownership and can not be NULL.
   In this case you should pass by reference
2. The function does not have ownership but may be NULL.
   Pass as a pointer and add comments that ownership is not being transferred. This is the worst situation and you should try and avoid this as the semantics are not clearly expressed by the code.
3. The function takes ownership.
   In this case I would suggest using std::auto_ptr as it explicitly indicates that ownership is being transferred to the function.
4. The function shares ownership.
   In this case some form of shared smart pointer like boost::shared_ptr or std::tr1::shared_ptr

In your case I would suggest 1 or 2 depending on the situation.
It is obviously that the function can not delete the pointer as it sometimes may not have ownership. Thus the ownership remains on the side of the caller. So the caller must call the delete as appropriate. Hopefully via some smart pointer mechanism.

Answer 6

You need to decide on the ownership, either:

1. Caller owns object: object is LOANED to function, Caller is responsible for allocation and deletion.

2. Function owns object: object is DONATED to function, Caller is responsible for allocation, callee (function) is responsible for deletion.

3. Use a reference counting via a shared pointer to avoid the problem.

( 2 is ugly because it means that different code is responsible for allocation and deletion, but that might be the correct solution for your case - it also means making copies of the argument if the caller wants to keep the object).

Answer 7

Both of these are valid options in the c++ syntax sense, but I'd argue that the second option is bad software design in almost all cases, and allowing both options in one function might even be bad software design in all cases.

Either the calling class creates the object, calls the method, and deletes the object,
or the calling class creates the object, calls the methon, and expects the method to delete the object.

I see no good reason why both options should be considered reasonable in one method call. It should be clear from documentation or comments which version the method uses, and it is then up to the developer to use that method, or suffer the consequences.

Answer 8

The common behavior is for functions neither to create nor to destroy objects. However, there are functions that do. Common jargon is "sources and sinks". A very useful convention, if you're not using more versatile smart pointers, is to use std::auto_ptr<T> for both sources and sinks. A "source" function returns the object created as an auto_ptr<T>, whereas a sink takes an argument of type auto_ptr<T>.

The benefit is that even if you forget the return value of a source function, you won't have a memory leak. At the end of the statement, the returned auto_ptr destroys the object returned. And similarly, it is obvious that the argument passed in to a void sink(auto_ptr<T> unreferenced) { } will be destroyed before sink returns.

Answer 9

The memory should be freed by its owner. Usually, the owner is the entity that allocated the memory, but in some cases you may allow ownership to be transferred.

void foo() {
  myClass* p = new myClass(); // foo owns the pointer, so foo should release it again
}

void foo() {
  boost::shared_ptr<myClass> p = new myClass(); // foo allocates the memory, but *transfers* ownership to the smart pointer. The smart pointer is now responsible for freeing the object. (This is true for all types of smart pointers, including boost::scoped_ptr, std::auto_ptr or the C++0x std::unique_ptr.
}

void foo() {
  ScopedMyClass x; // ScopedMyClass is some class wrapper which *internally* calls `new myClass`, and so owns the allocation and is responsible for freeing it before the end of the wrapper class' lifetime.
}

In your case specifically, if myMethod doesn't allocate the memory, then it should not free the memory either.

As for how to handle this:

myMethod(new myClass(...));

Just don't do that. If you allocate memory, you need to take ownership of it. Store the pointer first, so you can delete the memory:

myClass* p = new myClass(...);
myMethod(p);
delete p;

or even better, *don't dynamically allocate the object in the first place, eliminating the question of ownership:

myMethod(myClass(...));

Answer 10

I can't comment yet but .. While I would normally argue strongly for the 'creator owns it' paradigm (can you really prove anything else correct?) , I think that the insistence on having the

myMethod(new myClass(...));

usage pretty much requires ownership to be transferred. Perhaps it would be prudent to ask what you're attempting to accomplish with this usage, and could it be handled in another way? Like an overload that takes a reference to the temporary instead of a pointer?

myMethod(myClass& a) { return myMethod(&a); }

myMethod(myClass(...))

Answer 11

Well, I suppose I'll add my little stone to the edifice... though I am not sure it will even be read.

This is a matter of ownership of the memory, it is the role of your interface to convey this meaning: "Will this method take ownership or not".

To better express ownership, it is usually bad to use raw pointers. The STL has the bastard auto_ptr which is better suited to the task (I am waiting for the upcoming unique_ptr rather impatiently there).

For a method, there are several ways to accept parameters:

// Value
void method(T);

// Reference
void method(T&);

// Pointer
void method(T*);

// Smart Pointer
void method(std::auto_ptr<T>);

I skipped the cv-qualification bit because it's irrelevant there.

The point is that out of these 4 solutions, the intent is clear:

• Value / Reference / Pointer: the caller is responsible for the memory

• auto_ptr the method takes ownership of the memory, this means that the caller cannot use the variable afterward

  std::auto_ptr myT = std::auto_ptr(new T()); method(myT); assert(myT.get() == 0); // myT does not hold anything any longer!

Of course, that's why auto_ptr is a wild beast out there, since it does not respect the convention that a copying an object leaves the copied object apparently unchanged: normally all the public methods should give the same result before and after, reference counting being a corner case here if you expose it.

So, your interface should be clearer if whether or not the caller should expect the method to take ownership or not, a simple way is to use overloading.

void method(T*); // do something

void method(std::auto_ptr<T> p)
{
  method(p.get());
}

Easy enough, you are now clear on who handles the memory!

With the same reasonment you can also use this overloading trick to automatically check a pointer for nullity.

void method(T&); // do something
void method(T* p)
{
  if (p) method(*p); else throw NullPointer("method");
}

But I would avoid abusing the trick, you'll end up with thousands of methods.

So just remember: ownership semantic is best expressed in CODE rather than in comments.

Which you will immediately mix with Use RAII to manage resources which means here that you should never allocate memory to a raw pointer: use a smart pointer to express ownership of the resource, and a raw pointer to point to existing objects you do not own (and that might possibly be null, otherwise a reference is better ;) ).