views:

703

answers:

4

Is this program well-defined, and if not, why exactly?

#include <iostream>
#include <new>
struct X {
    int cnt;
    X (int i) : cnt(i) {}
    ~X() {  
            std::cout << "destructor called, cnt=" << cnt << std::endl;
            if ( cnt-- > 0 )
                this->X::~X(); // explicit recursive call to dtor
    }
};
int main()
{   
    char* buf = new char[sizeof(X)];
    X* p = new(buf) X(7);
    p->X::~X();  // explicit call to dtor
    delete[] buf;
}

My reasoning: although invoking a destructor twice is undefined behavior, per 12.4/14, what it says exactly is this:

the behavior is undefined if the destructor is invoked for an object whose lifetime has ended

Which does not seem to prohibit recursive calls. While the destructor for an object is executing, the object's lifetime has not yet ended, thus it's not UB to invoke the destructor again. On the other hand, 12.4/6 says:

After executing the body [...] a destructor for class X calls the destructors for X's direct members, the destructors for X's direct base classes [...]

which means that after the return from a recursive invocation of a destructor, all member and base class destructors will have been called, and calling them again when returning to the previous level of recursion would be UB. Therefore, a class with no base and only POD members can have a recursive destructor without UB. Am I right?

+1  A: 

Yeah, that sounds about right. I would think once the destructor is finished calling, the memory would be dumped back into the allocatable pool, allowing something to write over it, thus potentially causing issues with follow-up destructor calls (the 'this' pointer would be invalid).

However, if the destructor doesn't finish until the recursive loop is unwound.. it should theoretically be fine.

Interesting question :)

Stefan Valianu
+41  A: 

The answer is no, because of the definition of "lifetime" in §3.8/1:

The lifetime of an object of type T ends when:

— if T is a class type with a non-trivial destructor (12.4), the destructor call starts, or

— the storage which the object occupies is reused or released.

As soon as the destructor is called (the first time), the lifetime of the object has ended. Thus, if you call the destructor for the object from within the destructor, the behavior is undefined, per §12.4/6:

the behavior is undefined if the destructor is invoked for an object whose lifetime has ended

James McNellis
@James: Unrelated question but where can I download/view the standard's documentation?
Jacob
D'oh, didn't double-check the definition of *lifetime*. It sounds odd though, since it means that during a normal call to a destructor, I'm accessing members of an object whose lifetime has ended. But if it says so, it must be true.
Cubbi
It's technically undefined according to the standard, but it seems to me that almost any reasonable implementation would allow it, considering that a destructor is easily implemented as an ordinary member function call just before the object is deallocated.
Jon Purdy
@Jacob: If you want the current, official standard then [you have to buy it](http://stackoverflow.com/questions/81656/where-do-i-find-the-current-c-or-c-standard-documents/83763#83763). If you want the final committee draft of the upcoming C++ standard (which is not official or final or approved and has many, many differences), you can find that [on the WG21 website](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/); it's document number N3092 (near the bottom).
James McNellis
@James: Many thanks! Could never find a good link. And I guess I should've checked on SO first before Google :)
Jacob
+1 : for answering clearly to a weird question
neuro
@Jacob: you can google for ANSI 14882:2003 and follow the third link, or go here:http://openassist.googlecode.com/files/C%2B%2B%20Standard%20-%20ANSI%20ISO%20IEC%2014882%202003.pdf
Semen Semenych
@Semen: Thanks a lot!
Jacob
+7  A: 

Okay, we understood that behavior is not defined. But let's do small journey into what really happends. I use VS 2008.

Here is my code:

class Test
{
int i;

public:
    Test() : i(3) { }

    ~Test()
    {
        if (!i)
            return;     
        printf("%d", i);
        i--;
        Test::~Test();
    }
};

int _tmain(int argc, _TCHAR* argv[])
{
    delete new Test();
    return 0;
}

Let's run it and set a breakpoint inside destructor and let the miracle of recursion happen.

Here is stack trace:

alt text

What is that scalar deleting destructor? It is something that compiler inserts between delete and our actual code. Destructor itself is just a method, there is nothing special about it. It doesn't really release the memory. It is released somewhere inside that scalar deleting destructor.

Let's go to scalar deleting destructor and take a look at the disassembly:

01341580  mov         dword ptr [ebp-8],ecx 
01341583  mov         ecx,dword ptr [this] 
01341586  call        Test::~Test (134105Fh) 
0134158B  mov         eax,dword ptr [ebp+8] 
0134158E  and         eax,1 
01341591  je          Test::`scalar deleting destructor'+3Fh (134159Fh) 
01341593  mov         eax,dword ptr [this] 
01341596  push        eax  
01341597  call        operator delete (1341096h) 
0134159C  add         esp,4 

while doing our recursion we are stuck at address 01341586, and memory is actually released only at address 01341597.

Conclusion: In VS 2008, since destructor is just a method and all memory release code are injected into middle function (scalar deleting destructor) it is safe to call destructor recursively. But still it is not good idea, IMO.

Edit: Ok, ok. The only idea of this answer was to take a look at what is going on when you call destructor recursively. But don't do it, it is not safe generally.

Andrey
This is still not even remotely safe: even though the destructor is "just" a member function, each time it returns it calls the destructors of any base classes and member variables (this is easy to test).
James McNellis
Testing the output of something is not an indication of the legality of any bit of code. What you get, at best, is the certain behavior of a certain compiler on a certain bit of code, but that has nothing to do with C++ as a language.
GMan
+1 for the testing attitude. I can still not imagine how you can have such an idea.
neuro
@GMan, yes, i agree. Thats why i emphasised on VS 2008. Anyway i don't think that someone should call d-tor recursively.
Andrey
Oh no, not a one more answer "explaining" that UB "is safe" in some case. UB is not safe, period. The compiler is not required to produce code that behaves reasonably. Even though you see this behavior now, the compiler is not required to emit the same machine code tomorrow. I recently asked these two questions: http://stackoverflow.com/questions/3053904/ and http://stackoverflow.com/questions/3059917/ and the answers clearly explain that you can go to great lengths "proving" that UB "is safe in this case", but your "proof" will anyway be worth less then nothing.
sharptooth
@sharptooth original idea behind my answer was to take a look at what is going on when you call destructor recursively. it was not right to say that it is safe in conclusion/
Andrey
Come on guys. This spirit of experimentation and digging into what the compiler actually does is to be admired and encouraged, not criticised. He clearly doesn't recommend this in real life, and people knowing how C++ -> ASM is a _good thing_
John
Try making Test inherit from Base, and declare their dtors virtual. I think (not near a compiler at the moment) your test would crash. The compiler does more than destroy members: it destroys the instance vtable, replacing it with the parent's, *before* the dtor body: http://thetweaker.wordpress.com/2010/06/03/on-_purecall-and-the-overheads-of-virtual-functions/
Ofek Shilon
@Andrey: the problem is that you haven't actually proven that VS2008 implements it like this. All you've shown is that *at the time you compiled, on your specific hardware, with your specific sample code, it generated this code. There is no guarantee that the same compiler will do the same again. In particular, the compiler may apply completely unrelated, but safe, optimizations which just so happens to break code that relies on undefined behavior.That's an important caveat. Examining what the compiler actually does here is interesting, but it gives you no information you can rely on.
jalf
@jalf i totally agree. my initial idea was just to **take a look** inside the thing. just pure curiosity. i already told that my conclusions are not legitimate.
Andrey
+5  A: 

It comes back to the compiler's definition of the lifetime of an object. As in, when is the memory really de-allocated. I would think it could not be until after the destructor has completed, as the destructor has access to the object's data. Therefore, I would expect recursive calls to the destructor to work.

But ... there are surely many ways to implement a destructor and the freeing of memory. Even if it worked as I wanted on the compiler I'm using today, I would be very cautious about relying on such behavior. There are lots of things where the documentation says it won't work or the results are unpredictable that in fact work just fine if you understand what is really happening inside. But it's bad practice to rely on them unless you really have to, because if the specs say that this doesn't work, then even if it really does work, you have no assurance that it will continue to work in the next version of the compiler.

That said, if you really want to call your destructor recursively and this isn't just a hypothetical question, why not just rip the entire body of the destructor into another function, let the destructor call that, and then let that call itself recursively? That should be safe.

Jay
+1 for suggesting the standards-compliant alternative.
Jon Purdy
I cannot see what functionality that would allow that is not available from the dtor body itself - both by the standard and in practice. You're at the *exact* same place regarding the object lifetime. For example, you'd have to be very careful not to call virtual functions from that separated recursive call, as you'd get undefined behaviour.
Ofek Shilon
A destructor is not a normal function: You are not supposed to call a destructor directly: it is only supposed to be called by the "framework code" that manages object deallocation. Whether this means that a destructor can or cannot call itself recursively is, to the best of my knowledge, not clearly defined in the language specifications. It is possible that a compiler could implement destructors in a way that makes a recursive call not work, e.g. it might prematurely deallocate the object. So my point is: Why take chances? (continued)
Jay
(continued) It is guaranteed legal to call a recursive function from within the destructor. So you can easily achieve the effect desired by having the destructor call some other function which is recursive. Then any ambiguity in the language spec is irrelevant. If someone can point to something in the language spec that makes it clear that a recursive destructor is safe and legal, then yes, my suggestion here is unnecessary. But as I don't think that is the case, might as well play it safe. I wear my seat belt even on days when I don't plan to be in an accident.
Jay