C++ bad_alloc thrown in a constructor

Question

When a bad_alloc exception is thrown in a constructor, in which multiple objects are created, what must be done to clean up the memory. Ex.

class Object
{
   private:
     A * a;
     B  * b;

 public:

   Object()
   {
       a= new A();
       b= new B(); //So if a bad_alloc is called here, how is a deleted???
   }
}

My intuition is to place each call to new in a separate try catch bloc, and delete all objects for which new was called previously but this is too verbose (the first try bloc calls no destructor, the second class that of the first, the 3rd calls that of the first two etc). My question is: What is the most common way to handle this?

Also, lets say the class objects contains a object not created with new (because it is on the stack), is its destructor called automatically?

Implement the class to effect the "Rule of Zero". Here `std::unique_ptr` is an option. — Niall, Sep 19 '14 at 11:28
If `bad_alloc` is thrown when you allocate `a`, you can put it inside its own `try` block. `try {a = new A();} catch (...) {delete a;} try {b = new B();} catch (...) {delete a; delete b;};` could solve your problem. — rsethc, Sep 19 '14 at 11:50
But remember... unless you're developing a commercial or other large application, you can still get away with some memory leaks in general. And as for if you don't catch the exception, *all* memory attached to the application will be freed anyway, whether the application specifies it should be or not. — rsethc, Sep 19 '14 at 11:52
If you find yourself wanting to use a series of separate `try-catch` blocks or nested `try-catch` blocks you're doing something wrong. Learn to use RAII instead. Lots of manual exception-handling is fragile, tedious and error-prone, and very poor style. — Jonathan Wakely, Sep 19 '14 at 11:56
@rsethc, no, no, one thousand times no. That code is ugly, and not even correct. Why are you calling `delete a` when `a` never got initialized? That will try to delete a garbage pointer. Same problem for `delete b`. Also, because you swallow the exceptions when the constructor exits both pointers will be left containing garbage values. Go to jail, go directly to jail, do not pass Go, do not collect £200. (But thank you for proving why manual exception-handling should be avoided like the plague.) — Jonathan Wakely, Sep 19 '14 at 11:59
Actually, yes, it would be fine. `delete` does not care about garbage pointers, it only matters if there is the coincidence that it is equal to some other pointer you allocated that is actually valid. This, however, can be easily solved by setting the value to `NULL` in each `catch` block, since you will never allocate and receive `NULL`. — rsethc, Sep 19 '14 at 12:05
@rsethc, nonsense. `delete` doesn't care about **null** pointers, but it is undefined behaviour to `delete` an uninitialized pointer, which is what you are suggesting. Read your comment again: Initially `a` and `b` are uninitialized. You try to allocate an `A`. If it fails, you `delete a` (which is undefined behaviour). Then you try to allocate a `B`. If it fails you `delete a` (which is undefined behaviour if the first allocation failed and `a` was never initialized) then you `delete b` (which is undefined behaviour). Then the constructor exits, leaving both pointers in unknown states. — Jonathan Wakely, Sep 19 '14 at 12:10
Alright, perhaps it is thanks to my particular compiler I've seen garbage pointers just be ignored. But still, if you just add in `a = NULL;` and `b = NULL;` like so: `try {a = new A();} catch (...) {a = NULL;}; try {b = new B();} catch (...) {delete a; b = NULL;};`, then that should work. Also, yes, I do now realize I had said `delete a;` in the `a` exception block and `delete b;` in the `b` exception block, that was a mistake. Thanks for pointing it out. — rsethc, Sep 19 '14 at 12:14
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/61558/discussion-between-jonathan-wakely-and-rsethc). — Jonathan Wakely, Sep 19 '14 at 12:15

Paul Evans · Answer 1 · 2014-09-19T11:26:15.643

6

You want to use smart pointers:

class Object {
    std::unique_ptr<A> a;
    std::unique_ptr<B> b;

public:
    Object() : a(make_unique<A>()), b(make_unique<B>()) {}
}

edited Sep 19 '14 at 11:26

answered Sep 18 '14 at 16:55

Paul Evans

27,315
3
37
54

1

This works because of the explanation here: See: http://stackoverflow.com/questions/9122331/how-does-raii-work-when-a-constructor-throws-an-exception – Scott Langham Sep 18 '14 at 16:58
Why the calls to `release()`? – interjay Sep 18 '14 at 16:59
Agreed with interjay, I also don't think `.release()` should be there. `make_unique()` returns a `unique_ptr ` rvalue, so can be move-assigned to `a`. – Sep 18 '14 at 17:01

CashCow · Answer 2 · 2014-09-19T11:53:07.363

Firstly I fixed your code because it is a C++ question, so it has to be written as C++. A constructor might fail with exceptions other than bad_alloc.

Your options are there:

Do not store pointers but store the objects. These are constructed automatically (or via the initialiser list) and yes will automatically be cleaned up if created. This can be better but means they need to be fully defined, i.e. their headers included, and you may be trying to hide your implementation detail / decouple..
Store some kind of smart pointer like unique_ptr which is really an object so gets cleaned up like an object, deleting the underlying pointer if there is one.
Store a regular pointer in your class but use unique_ptr (or auto_ptr if unique_ptr is not available) during the constructor and, at the end when you know everything has constructed properly, you can release your smart pointers into your member variables.

The latter solution would look like:

 // header file
 //
class A; // implementation hidden on purpose
class B; // ditto

class Object
{
   private:
      A * a;
      B * b;

   public:
     Object();
     ~Object();

    private:

 // if not using C++11 do not put in =delete but still declare these

    Object( const Object & ) = delete;
    Object& operator=( const Object & ) = delete;

};

// cpp file

#include "object.h"
#include "a.h"
#include "b.h"

Object::Object()
   : a( nullptr ), // use NULL or 0 if nullptr not available
     b( nullptr )
{
    std::unique_ptr< A > pa( new A ); // might throw
    std::unique_ptr< B > pb( new B ); // might throw (1)

    a = pa.release(); // cannot throw
    b = pb.release(); 
}

Object::~Object()
{
     delete b;
     delete a;
}

(1) if it throws pa which is local will have its destructor invoked which will delete the pointer you created with new.

Note: if you don't have unique_ptr available, auto_ptr will serve just as well here.

C++ bad_alloc thrown in a constructor

2 Answers2