views:

138

answers:

6

Say for example i have the following code (pure example):

class a {
   int * p;
public:
   a() {
      p = new int;
   }
   ~a() {
      delete p;
   }
};

a * returnnew() {
   a retval;
   return(&retval);
}

int main() {
   a * foo = returnnew();
   return 0;
}

In returnnew(), would retval be destructed after the return of the function (when retval goes out of scope)? Or would it disable automatic destruction after i returned the address and i would be able to say delete foo; at the end of main()? Or, in a similar vein (pseudocode):

void foo(void* arg) {
   bar = (a*)arg;
   //do stuff
   exit_thread();
}

int main() {
   while(true) {
      a asdf;
      create_thread(foo, (void*)&asdf);
   }
   return 0;
}

where would the destructor go? where would i have to say delete? or is this undefined behavior? Would the only possible solution be to use the STL referenced-counted pointers? how would this be implemented?

Thank you- i've used C++ for a while but never quite been in this type of situation, and don't want to create memory leaks.

+9  A: 

For a stack created object, the destructor is automatically called when the object goes out of scope.

For an object created on the heap, the memory will be freed only when you explicitly call delete.

Whether you return the address of a stack created object from a function or not does not matter. The destructor will still be called when the item goes out of scope.

So for your code example:

a * returnnew() 
{
   a retval;
   return(&retval);
}

a's destructor is called before the code jumps back to the code that calls returnnew(). You return the address of that object, but that address is pointing to a place in memory that no longer belongs to you.

Where would i have to say delete?

You only use delete when you used new
You only use delete[] if you used new[]

or is this undefined behavior?

What you do with the address of memory that doesn't belong to you will be undefined behavior. It is not correct code though.

Would the only possible solution be to use the STL referenced-counted pointers?

You could return the object by value, or you could create a new object on the heap. You can also pass in the object to the function via a parameter and ask the function to change it.

how would this be implemented?

//Shows how to fill an object's value by reference
void fillA(a& mya) 
{
   mya.var = 3;
}

//Shows how to return a value on the heap
a* returnNewA() 
{
  //Caller is responsible for deleting the returned pointer.
  return new a();
}

//Shows how to return by value, memory should not be freed, copy will be returned
a returnA() 
{
  return a();
}
Brian R. Bondy
+1 for nice explanation
Yogesh Arora
Also note that the "return by value" option will be optimized by any decent compiler so that it doesn't create any unnecessary copies, making it the best option in this case (and in general). http://en.wikipedia.org/wiki/Return_value_optimization
Josh Townzen
A: 

returnnew() will destroy the variable upon return because you're returning a pointer to a local variable. If you want to return retval from there, allocate it dinamically, eg:

a * returnnew() {
    a * retval = new a;
    return retval;
}

Or just:

a * returnnew() {
    return new a;
}

Dynamic allocated memory has no scope, and thus won't be deallocated until you say so, by means of a delete/delete[]/free, or until the program exits (and few other cases not related to the question). Before anyone comments here, my "until you say so" also includes shared/smart/etc pointer behaviour.

As for your second code & questions, if you are allocating the variable outside the thread, but ONLY using it from inside, you can just make the thread deallocate (delete) it when it's no longer needed. However, if you plan to access the variable from multiple points, and you can't guess when you're safe to destroy it, surely, use a smart or shared pointer.

jweyrich
A: 

+1 to Brian's answer, just want to add a comment about the threading aspect.

Your code to create a thread is going to destroy the asdf object that you passed to the thread function regardless of the child, because asdf is on the parent stack. Either create asdf on the heap, or pass by value. Otherwise, the parent will destroy asdf, and leave your child thread pointing to a bad address on the parent's stack. Not good in any case, destructors or no destructors. The only way you'd safely pass asdf to a function in a thread would be if you created the thread first, and asdf was a stack object in ITS stack, not the parent stack.

void foo(void* arg) { 
   bar = (a*)arg; // child has reference to a parent stack address!
   //do stuff 
   exit_thread(); 
} 

int main() { 
   while(true) { 
      a asdf; // parent stack
      create_thread(foo, (void*)&asdf); // parent and child diverge here, asdf auto-destroyed
   } 
   return 0; 
} 
mrjoltcola
+1  A: 
a * returnnew() {
   a retval;
   return(&retval);
}

Here, retval has automatic storage duration, which means that the language will automatically destruct it when it goes out of scope. The address you've returned refers to an object that no longer exists, and trying to use the return value will be an error.

When you want an object's lifetime to be controlled by you, you have to use the new operator to create it.

a* returnnew() 
{ 
   a* retval = new a();  
   return retval;  
}

Here, you now have complete control over the lifetime of this a. It will live until you explicitly delete it, or your program ends.

You could also come up with meaningful copy semantics for your a class, and return it by value, in which case your caller would get his own copy, distinct from the original. Then, your caller doesn't care when the original goes away.

class a 
{
   int * p;
public:
   a(a const& rhs) 
   {
      p = new int(rhs.p)
   }
   a() 
   {
      p = new int;
   }
   ~a() 
   {
      delete p;
   }
};

Now, you can construct a fresh a as a copy of an existing a. So your function could then return an a by value, like so:

a returnnew() 
{ 
   a retval;  
   return retval; 
}

Here retval's lifetime will end when the function returns, and it will be destructed automatically by the language, and no resources will be leaked. And your caller will have his own copy, with its own lifetime.

In my experience, most classes should have sensible copy semantics, and you should not be afraid to pass and return them by value. It's just simpler that way, and you'll avoid dangling pointer problems.

One of C++'s biggest strengths is the way that destructors are automatically called by the language when automatic storage duration objects go out of scope. If you ensure that every resource in your program is owned by such an object, you'll have a much harder time leaking resources.

janks
You need an assignment operator that does `*p = *(rhs.p)`.
FredOverflow
Woops, that's a good call. You nearly always want neither or both.
janks
A: 

In the case of the following code:

void foo(void* arg) {
   bar = (a*)arg;
   //do stuff
   exit_thread();
}

int main() {
   while(true) {
      a asdf;
      create_thread(foo, (void*)&asdf);
   }
   return 0;
}

The destructor is called at the closing brace of the while loop. This means it will be called every iteration of the loop (and the will be constructed again for the next iteration).

As a useful tool in exploring the nuances of constructors and destructors and scopes, consider using the following class to help you answer these questions by yourself in the future:

class trace {
private:
  std::string msg_;
public:
  explicit trace(const std::string &msg) : msg_(msg) {
    std::cerr << "Constructing: " << msg_ << std::endl;
  }
  ~trace() {
    std::cerr << "Destructing: " << msg_ << std::endl;
  }
};

Use it thusly:

trace glb("global");

main() {
  trace t1("top of main");

  for(int i = 0; i < 10; ++i)
  {
    trace t2("inside for");
  }

  return 0;
}

The results may surprise you.

SoapBox
+1  A: 

As a general rule the easiest way to avoid memory leaks in C++ is to avoid using new and delete as much as possible.

If you must have a pointer to something use a smart pointer (such as a boost scoped_ptr or shared_ptr). This way you can still have your object on the heap, but it's deconstructor will be called when the smart pointer goes out of scope. Otherwise trying to be sure that you call delete in every case can be a headache and cause a lot of extra code.

J Higs