"Stealing Code from Destructor" bug

ricecake

I saw this post on comp.lang.c++, and thought it would be good to share with this community (note: I am not the original author, but I have added some HTML markup to the original post): The "stealing code from the destructor" bug[^]

I've been bitten twice now by the same bug, and so I thought I would draw it to people's attention to try to save others the problems I've had. The bug arises when you copy code from a destructor to use elsewhere. For example, suppose you have a class Note. This class stores some text, as a linked list of lines of text. The destructor runs as follows:

Note::~Note() {
Line *lin, *lin2;

lin = firstline;
while (lin) {
lin2 = lin;
lin = lin -> next;
delete lin2; }
}

which works fine. You now decide it would be nice to be able to clear the text of a note, using this code, so you split it up as follows:

Note::~Note() {
cleartext();
}

void Note::cleartext() {
Line *lin, *lin2;

lin = firstline;
while (lin) {
lin2 = lin;
lin = lin -> next;
delete lin2; }
}

So now you have an extra function you can use. And the destructor is calling a neat, modular function, which is arguably better style than the destructor being a sprawl of code. The snag is that your program then mysteriously goes wrong. Worse still, because the above is only a minor change to code that was working perfectly, you tend to assume that the problem must instead lie in the other changes you made at the same time - the code that uses cleartext. For those who haven't spotted the snag, the problem is this. The destructor didn't try to leave the Note in a usable state, because the Note was not going to be used again. Cleartext does however need to leave the Note in a usable state, so it needs a line:

firstline = NULL;

to be added at the end. In fact, it also needs lastline = NULL; and a few other additions. Is this a bug that has been described before? If not, do I get to name it? I was originally going to call it the "nicking code from the destructor" bug but seeing as these newsgroups have an international audience I think I will go for the "stealing code from the destructor" bug. I've only hit the bug in C++ but

Ryan Binns

I always write my destructors so that they leave the object in a reuseable state as much as possible, mostly to avoid exactly this situation. The other reason is that destructors can be called at any time just like any other method, not only when it is destroyed.

myObject.~MyClass();

Works just fine.

Ryan

"Punctuality is only a virtue for those who aren't smart enough to think of good excuses for being late" John Nichol "Point Of Impact"

Lost User

Good one. Also, IIRC, PC Lint can spot this and warn you to NULL pointers in destructors, etc.

Kicking squealing Gucci little piggy.

ricecake

Ryan Binns wrote:

The other reason is that destructors can be called at any time just like any other method, not only when it is destroyed.

myObject.~MyClass();

Works just fine.

You can do that, but I think it's only safe when myObject was created using placement new. If you try to do that on a stack-allocated variable, then the destructor will get called again when the variable goes out of scope. If you try to invoke the destructor on a heap-allocated variable with myObject->~MyClass();, then it should only be safe if myObject is created using manual memory management (e.g., using malloc()) and placement new (new(myObject) MyClass;), and then the memory must be manually freed after the destructor was manually called. I prefer to let the language semantics handle all this for me though. :)

-- Marcus Kwok

Ryan Binns

ricecake wrote:

I prefer to let the language semantics handle all this for me though.

Oh so do I. I never call the destructor manually, although I have seen it done in some wierd cases (like MFC's CArray - or was that only the constructor, not the destructor, I can't remember, though I think it's both).

Ryan

"Punctuality is only a virtue for those who aren't smart enough to think of good excuses for being late" John Nichol "Point Of Impact"

Chris Losinger

ugh. i've made that mistake more than once. destructors always endup being gutted so that i can have a "CleanUp" function, but then forget that the dtor can make assumptions about the lifespan of member objects that a CleanUp function shouldn't. yuck.

image processing | blogging

Mike Dimmick

CArray uses manual calls of the destructor to ensure that when you call Remove, the number of valid elements is actually what you expect. Otherwise, a 'dead' object hangs around, potentially not freeing memory that it owns. Similarly, it uses 'placement new' to create objects at a specific memory location. It's doing this to avoid having to constantly reallocate the base array and copy the objects repeatedly. Instead, it can grow into existing free space and reduce the number of calls to the allocator.

Stability. What an interesting concept. -- Chris Maunder

Ryan Binns

Yeah, that's what I thought :)

Ryan

"Punctuality is only a virtue for those who aren't smart enough to think of good excuses for being late" John Nichol "Point Of Impact"