20

In the following code:

#include <memory>
#include <iostream>

void mydeallocator(int * x) {
    std::cerr << "Freeing memory" << std::endl;
    delete x;
}

struct Foo {
    std::unique_ptr <int,std::function <void(int*)>> x;
    Foo(bool fail) : x(new int(1),mydeallocator) {
        if(fail)
            throw std::runtime_error("We fail here");
    }
};

int main() {
    {auto foo1 = Foo(false);}
    {auto foo2 = Foo(true);}
}

It appears that memory is not being deallocated properly when Foo(true) is called. Namely, when we compile and run this program, we have the result:

Freeing memory
terminate called after throwing an instance of 'std::runtime_error'
  what():  We fail here
Aborted

I believe that the message Freeing memory should be called twice. Basically, according to this question and the ISO C++ folks here and here, my understanding is that the stack should unwind on the constructor for Foo and that x should call its destructor, which should call mydeallocator. Certainly, this is not happening, so why is the memory not being freed?

wyer33
  • 6,060
  • 4
  • 23
  • 53
  • 2
    @TheParamagneticCroissant Foo's constructor is throwing the exception, not the constructor of `unique_ptr`. Since `x` has been fully constructed when the exception is thrown the destructor of `unique_ptr` should be invoked for `x`. – Captain Obvlious Jul 24 '15 at 05:20
  • 2
    @TheParamagneticCroissant `~Foo()` would not be run, but `~unique_ptr()` should be - no? – Jonathan Potter Jul 24 '15 at 05:20
  • It's not just that, because you're not catching the exception, you're losing the console output due to the program aborting? – Jonathan Potter Jul 24 '15 at 05:21
  • 1
    Oh, wait, you `throw;` without an active exception. That goes straight to `terminate`. No stack unwinding happens at all. – T.C. Jul 24 '15 at 05:32
  • @T.C. The same error will occur with a runtime_error. One moment and I'll fix the question. – wyer33 Jul 24 '15 at 05:36
  • 3
    You do not catch the exception, so the execution goes to `terminate()` before the unique_ptr is freed. If you add a `catch` handler you will see both messages. – M.M Jul 24 '15 at 05:41
  • BTW: If you don't really need the flexibility of using a `std::function`, consider reducing it to a simple pointer. Or preferably an empty function-object. See http://stackoverflow.com/questions/28413035 for a function making that easier. – Deduplicator Jul 24 '15 at 12:53
  • If an exception escapes `main()` then it is implementation defined if the stack is unwound. That's why you should catch everything in main() and log/rethrow (if you want to force stack unwinding). – Martin York Jul 24 '15 at 17:57

1 Answers1

21

Your original code throw; when you have nothing to rethrow. That causes std::terminate to be called; the stack is not unwound (and hence the destructors don't run).

Your new code throws an exception without handling it. In that case whether the stack is unwound is implementation-defined, so it's still perfectly conforming to terminate() right away. [except.terminate], emphasis mine:

In some situations exception handling must be abandoned for less subtle error handling techniques. [ Note: These situations are:

  • when the exception handling mechanism, after completing the initialization of the exception object but before activation of a handler for the exception (15.1), calls a function that exits via an exception, or
  • when the exception handling mechanism cannot find a handler for a thrown exception (15.3), or
  • when the search for a handler (15.3) encounters the outermost block of a function with a noexcept-specification that does not allow the exception (15.4), or
  • when the destruction of an object during stack unwinding (15.2) terminates by throwing an exception, or
  • when initialization of a non-local variable with static or thread storage duration (3.6.2) exits via an exception, or
  • when destruction of an object with static or thread storage duration exits via an exception (3.6.3), or
  • when execution of a function registered with std::atexit or std::at_quick_exit exits via an exception (18.5), or
  • when a throw-expression (5.17) with no operand attempts to rethrow an exception and no exception is being handled (15.1), or
  • when std::unexpected exits via an exception of a type that is not allowed by the previously violated exception specification, and std::bad_exception is not included in that exception specification (15.5.2), or
  • when the implementation’s default unexpected exception handler is called (D.8.1), or
  • when the function std::nested_exception::rethrow_nested is called for an object that has captured no exception (18.8.6), or
  • when execution of the initial function of a thread exits via an exception (30.3.1.2), or
  • when the destructor or the copy assignment operator is invoked on an object of type std::thread that refers to a joinable thread (30.3.1.3, 30.3.1.4), or
  • when a call to a wait(), wait_until(), or wait_for() function on a condition variable (30.5.1, 30.5.2) fails to meet a postcondition. —end note ]

In such cases, std::terminate() is called (18.8.3). In the situation where no matching handler is found, it is implementation-defined whether or not the stack is unwound before std::terminate() is called. In the situation where the search for a handler (15.3) encounters the outermost block of a function with a noexcept-specification that does not allow the exception (15.4), it is implementation-defined whether the stack is unwound, unwound partially, or not unwound at all before std::terminate() is called. In all other situations, the stack shall not be unwound before std::terminate() is called. An implementation is not permitted to finish stack unwinding prematurely based on a determination that the unwind process will eventually cause a call to std::terminate().

T.C.
  • 133,968
  • 17
  • 288
  • 421
  • What's the right way to free the memory then? If I add a try-catch block inside the constructor that rethrows the exception, we still leak. What I'd like to have happen is for the memory to be freed even when an exception is called regardless if my user writes a try-catch block around the initialization of the class. – wyer33 Jul 24 '15 at 05:51
  • 1
    @wyer33 If the user doesn't catch the exception, the whole program is going to crash and burn, so why do you care if the memory is freed? – T.C. Jul 24 '15 at 06:10
  • @wyer33 Try/catch in main –  Jul 24 '15 at 06:10
  • 1
    @T.C. Well, in my actual case, I've a pointer to a database and I'd like to clean up some entries before the program terminates. Normally, I do that with a custom deallocator on a unique_ptr. – wyer33 Jul 24 '15 at 06:14
  • 2
    @wyer33 OK, then call `reset()` on the `unique_ptr` to force clean-up before you rethrow the exception. – T.C. Jul 24 '15 at 06:16
  • 1
    @T.C. Alright, that works. Also, what's odd is that if I use a function-try-block `Foo(bool fail) try : x(new int(1),mydeallocator) {...} catch(...) { throw; }`, everything deallocates properly. Is this reliable or does that fall into the implementation defined unwinding? Note, the same does not occur if I use a try-catch block inside the constructor. The function-try-block is required. – wyer33 Jul 24 '15 at 06:20
  • 3
    @wyer33 Yes, you can rely on the function-try-block. All fully-constructed subobjects are destroyed before entering its handler. – T.C. Jul 24 '15 at 06:25
  • 2
    @T.C. Thanks for the help! In case someone else is looking, I believe that the relevent section is 15.3.11 `The fully constructed base classes and members of an object shall be destroyed before entering the handler of a function-try-block of a constructor for that object.` – wyer33 Jul 24 '15 at 06:37