0

In this code, if the if clause is true, an exception is thrown from the CurrentAccount constructor:

void Bank::createAccount(string accountType,int iban,int ownerid,double amount)
{
    Account* toAddAccount=nullptr;
    if(accountType=="CurrentAccount")
    {
        toAddAccount=new CurrentAccount(iban,ownerid,amount);
    }
}

As you can see, the exception is not caught in this method, but is promoted higher on the stack.

I was wondering, will there be memory leak since I don't delete toAddAccount (the CurrentAccount constructor works with ints only)?

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Karmen
  • 1
  • 3
  • 2
    No, the CurrentAccount is never created, because the constructor never runs to completion, so there is nothing to delete. –  Jun 08 '18 at 20:03
  • 1
    There is no leak when the exception is thrown, as it will abort the construction of the `CurrentAccount` object. Since there is no `CurrentAccount` object fully created, it does not need to be freed later. There is, however, a leak if the `CurrentAccount` constructor does not throw an exception, since you are not deleting `toAddAccount`... – Remy Lebeau Jun 08 '18 at 20:04
  • If you use `std::unique_ptr` you don't have to worry about a memory leak.. – JHBonarius Jun 08 '18 at 20:06
  • @JHBonarius That would be true if the `CurrentAccount` constructor does not throw. If it throws, there is no object to put into the `std::unique_ptr` – Remy Lebeau Jun 08 '18 at 20:08
  • 1
    More to the point, if the constructor throws then the code for operator new invokes a matching operator delete to release the allocated memory block (the destructor is not called (as the object doesn't 'exist') but operator delete is). – SoronelHaetir Jun 08 '18 at 20:10
  • 1
    @RemyLebeau true. More of a general thing to promote the use of smart pointers... – JHBonarius Jun 08 '18 at 20:13

2 Answers2

3

It's not a leak because "new expression" is responsible for cleaning up if during its execution an exception is thrown. In other words, "new expression" allocates memory and then calls CurrentAccount's constructor. If this constructor throws the "new expression" automatically deallocates previously allocated memory.

navyblue
  • 776
  • 4
  • 8
  • 2
    It should also be noted that any class members that are *successfully* constructed by the compiler *before* the exception is thrown will be automatically destructed by the compiler. However, any members that are allocated with `new` before the exception is thrown need to be `delete`'d (via `try/catch`, smart pointers, etc) before the exception escapes the constructor, or else they will be leaked. – Remy Lebeau Jun 08 '18 at 20:16
  • 2
    Which is why you should always use RAII. –  Jun 08 '18 at 20:28
0

I think this question has already been answered, but you should be using RAII when building your objects. Specifically (as has been pointed out several times already) is the use of std::unique_ptr (and make_unique depending on your standard).

Account* Bank::createAccount(string accountType,int iban,int ownerid,double amount)
{
    std::unique_ptr<Account> toAddAccount;
    if(accountType=="CurrentAccount")
    {
        toAddAccount=new CurrentAccount(iban,ownerid,amount);
    }

    // presumably more code

    return toAddAccount.release(); // your factories shouldn't care where your accounts are stored.
}
James Poag
  • 2,320
  • 1
  • 13
  • 20
  • In this specific case, there is no point in using unique_ptr.. The CurrentAccount object is never constructed, so there is nothing to destroy, and the memory allocated for it is freed by the compiler generated code. –  Jun 08 '18 at 20:32
  • I agree. In the particular edge case of the OP question, there is nothing to destroy. In general practice, this is how you would use RAII techniques to prevent memory leaks for when the constructor does complete and the fault lies elsewhere. – James Poag Jun 08 '18 at 20:34