"delete this" question

[Christopher Benson-Manica]

Is the following code legal, moral, and advisable?

#include <iostream>

class A {
private:
int a;

public:
A() : a(42) {}
~A() {
std::cout << "A's destructor called, a is " << a << std::endl;
}
};

class B : public A {
private:
int b;

public:
B() : b(666) {}
~B() {
std::cout << "B's destructor called, b is " << b << std::endl;
}
void go() { delete this; }
};

int main() {
B *b=new B();
b->go();
return 0;
}

Specifically, are B and A's destructors still permitted to access
their class' member variables after the this pointer has been deleted?

 

[Rolf Magnus]

Is the following code legal, moral, and advisable?

#include <iostream>

class A {
private:
int a;

public:
A() : a(42) {}
~A() {
std::cout << "A's destructor called, a is " << a << std::endl;
}
};

class B : public A {
private:
int b;

public:
B() : b(666) {}
~B() {
std::cout << "B's destructor called, b is " << b << std::endl;
}
void go() { delete this; }
};

int main() {
B *b=new B();
b->go();
return 0;
}

Specifically, are B and A's destructors still permitted to access
their class' member variables after the this pointer has been deleted?

No, they aren't. But why would they need to? In your code, the destructors
are - just as usual - not called after the deletion, but as part of it.
And yes, you are allowed to delete an object in one of its member functions
as long as you make sure that nothing of it is accessed afterwards (i.e. in
your case, go() doesn't use any members of the object after the delete).

Whether 'delete this;' is a good design - well, I'd say that depends on
where and how the class is used.

 

[Alf P. Steinbach]

* Christopher Benson-Manica:

Is the following code legal, moral, and advisable?

Yes, whatever, no.

#include <iostream>

class A {
private:
int a;

public:
A() : a(42) {}
~A() {
std::cout << "A's destructor called, a is " << a << std::endl;
}
};

Intended for polymorphism but non-virtual destructor: bad.

class B : public A {
private:
int b;

public:
B() : b(666) {}
~B() {
std::cout << "B's destructor called, b is " << b << std::endl;
}
void go() { delete this; }
};

Different lifetime management policies for A and B: bad.

B supports copying but requires dynamic allocation: bad.

I/o in non-i/o classes: bad.

int main() {
B *b=new B();
b->go();
return 0;
}

Specifically, are B and A's destructors still permitted to access
their class' member variables after the this pointer has been deleted?

No, but they don't.

 

[Phlip]

Is the following code legal, moral, and advisable?

void B::go() { delete this; }

B *b=new B();
b->go();

C++ defines the act of returning from a method after its object deletes.

Whether its moral and advisable is a team and project decision. Some
frameworks encapsulate all their owner pointers, so the caller rarely sees
'new' and never sees 'delete'. Within this pattern, objects often must
delete themselves, and they notify all the linking pointers to NULL
themselves.

Now let's make your code illegal:

#include <iostream>

class B;

class A {
private:
int a; B & m_b;

public:

A(B &b) : a(42), m_b(b) {}
~A();

};

class B : public A {

public:
int b;

public:
B() : b(666) {}
~B() {
std::cout << "B's destructor called, b is " << b << std::endl;
}
void go() { delete this; }
};

A::~A() {
std::cout << "A's destructor called, a is " << a << std::endl;
std::cout << "B has already destructed, so accessing b is illegal "
<< m_b.b << std::endl;
}

int main() {
B *b=new B();
b->go();
return 0;
}

During destruction, the derived class sub-object destructs first, so
accessing its members is undefined during the split second before the entire
object's storage releases. In practice, our 'b' will most likely return the
correct value.

Specifically, are B and A's destructors still permitted to access
their class' member variables after the this pointer has been deleted?

Their _own_ class's members, yes. They can also call methods. Destruction
happens just before releasing.

 

[Christopher Benson-Manica]

Whether 'delete this;' is a good design - well, I'd say that depends on
where and how the class is used.

<ot>Well, what I have in mind is a class that takes a function pointer
and puts it on a background thread; when the function is complete, the
class will take care of deleting itself and stopping the thread.
Sound reasonable?</ot>

 

[Christopher Benson-Manica]

Yes, whatever, no.

Why do I feel like I'm conversing with an elf...?

<ot>I don't mind playing the role of Merry/Pippin - I really do look

Different lifetime management policies for A and B: bad.

So you're saying that if a given class never deletes itself, that no
subclass of that class should do so either...?

B supports copying but requires dynamic allocation: bad.

Is this a "Rule of Three" violation?

I/o in non-i/o classes: bad.

Explain? Please?

 

[Phlip]

Well, what I have in mind is a class that takes a function pointer
and puts it on a background thread; when the function is complete, the
class will take care of deleting itself and stopping the thread.
Sound reasonable?

Objects that wrap Win32 windows often respond to WM_CLOSE with 'delete
this'.

The distinction is these systems have an asynchronous event queue, so the
event triggering suicide comes from outside normal control flow.

But please don't thread if there are any alternatives!

 

[Phlip]

Elf P. Steinbach wrote:

So you're saying that if a given class never deletes itself, that no
subclass of that class should do so either...?

I disagree. Let's apply the Liskov Substitution Principle.

If 'go()' were virtual and inherited, then users of A * p might call
p->go(), then expect p to remain valid. When p really points to a B, p->go()
would delete it. This would force the caller to become aware of which
subtype of A it used. This violates LSP, which states that child classes
should not surprise clients of parent classes who want to remain unaware
they are using the child class.

However, 'go()' is not virtual. Users of A cannot call 'go()', and users of
B cannot believe they have an A. So LSP does not apply, and there are no
surprises.

There are situations where all members of an inheritance graph should use
the same creation strategy. For example, OO designs often revolve around big
sessile objects that should never copy, and little value objects that copy
around freely. But I think this one 'go()' is the exception that proves the
rule.

Is this a "Rule of Three" violation?

Bleah. He's just nitpicking. You did not turn off B's copy constructor.

And B does not "require" dynamic allocation, because you could create B on
the stack and then decline to call go().

Explain? Please?

Nitpicking again. You wrote 'cout' inside a class that should not surprise
clients with a side-effect that the user can see. Big friggin' deal.

 

[Rolf Magnus]

<ot>Well, what I have in mind is a class that takes a function pointer
and puts it on a background thread; when the function is complete, the
class will take care of deleting itself and stopping the thread.
Sound reasonable?</ot>

If the calling thread never needs any data of the called thread (maybe an
exit status or some calculated data stored in the thread object), yes.

 

[Howard]

Is the following code legal, moral, and advisable?

Hmmm... what exactly would "immoral" C++ code be?

(Sorry, couldn't resist.)

-H

 

[Christopher Benson-Manica]

I disagree. Let's apply the Liskov Substitution Principle.

I feel better that I apparently inadvertently followed it, but I
appreciate knowing that it exists in any case.

Bleah. He's just nitpicking. You did not turn off B's copy constructor.

I don't mind nitpicks, if I understand the reasoning behind them.

And B does not "require" dynamic allocation, because you could create B on
the stack and then decline to call go().

B would require some documentation letting users know how it was
intended to be used, however. Maybe the way to do it (what I really
want to do) would be to wrap the class that destroys itself in a class
that doesn't care, making it impossible to misuse it?

Nitpicking again. You wrote 'cout' inside a class that should not surprise
clients with a side-effect that the user can see. Big friggin' deal.

Again, I don't mind nitpicks, especially since I know I'm living in a
bubble of sorts at my present place of employement...

 

[Christopher Benson-Manica]

But please don't thread if there are any alternatives!

Why do you say that? (non-rhetorical question)

 

[Phlip]

Why do you say that? (non-rhetorical question)

I research GUI architectures. One of the most common anti-patterns in this
space is "that button takes too long, and locks up the GUI while it runs, so
we made the button launch a thread".

Because I don't research OS kernels, embedded software, or other primitive
spaces that need threads to get anything done, I have never seen a situation
improved by a thread. Throwing a thread at a simple problem is worse than
'goto'. If a button starts a process that takes too long, the process itself
needs a better architecture.

Suppose I write a function that finds each file on your hard drive, and
inspects it for virii. The main work loop recurses, following the folders
like this:

click the button
open a folder
open a sub folder
read a file
is it infected?

That's going to take forever. While the button serves its click event, the
main GUI event queue cannot dispatch other messages, so the user perceives a
locked-up GUI.

Now let's upgrade this design, to decouple it and make it event driven:

click the button
change the button label to "cancel scan"
post a WM_TIMER message

on WM_TIMER message
fetch the next file name
if there's a next file
is it infected?
post a WM_TIMER message

The part "fetch the next file name" now uses a linked list of opened folders
to pull out the next one. This strategy decouples the act of traversing the
folder hierarchy from the act of inspecting for virii. Decoupling is good.
This strategy uses no threads, and uses a windows timer to cleanly start and
stop the scan. (Stop the scan by cancelling the current WM_TIMER message.)

The strategy is "event driven" because it doesn't store state in the current
location of control flow. The recursive function stores the current folder
state locally, on the stack, and coupled to the file scanner. The event
driven system stores the current folder state explicitely in a linked list.
This, in turn, makes the folder traversal system modular and reusable.

 

[Mathias Waack]

I research GUI architectures. One of the most common anti-patterns in this
space is "that button takes too long, and locks up the GUI while it runs,
so we made the button launch a thread".

Because I don't research OS kernels, embedded software, or other primitive
spaces that need threads to get anything done, I have never seen a
situation improved by a thread. Throwing a thread at a simple problem is
worse than 'goto'. If a button starts a process that takes too long, the
process itself needs a better architecture.

I agree with Philip. Threads introduce a another level of complexity, thus
overstraining most developers. You have to change your way of thinking
before you can get a real gain by using threads.

Suppose I write a function that finds each file on your hard drive, and
inspects it for virii. The main work loop recurses, following the folders
like this:

click the button
open a folder
open a sub folder
read a file
is it infected?

That's going to take forever. While the button serves its click event, the
main GUI event queue cannot dispatch other messages, so the user perceives
a locked-up GUI.

Now let's upgrade this design, to decouple it and make it event driven:

click the button
change the button label to "cancel scan"
post a WM_TIMER message

on WM_TIMER message
fetch the next file name
if there's a next file
is it infected?
post a WM_TIMER message

The part "fetch the next file name" now uses a linked list of opened
folders to pull out the next one. This strategy decouples the act of
traversing the folder hierarchy from the act of inspecting for virii.
Decoupling is good. This strategy uses no threads, and uses a windows
timer to cleanly start and stop the scan. (Stop the scan by cancelling the
current WM_TIMER message.)

The strategy is "event driven" because it doesn't store state in the
current location of control flow. The recursive function stores the
current folder state locally, on the stack, and coupled to the file
scanner. The event driven system stores the current folder state
explicitely in a linked list. This, in turn, makes the folder traversal
system modular and reusable.

Of course its necessary to obtain the linked list of files (in the same
manner). And we silently assume, that "is it infected" will never block
(block from the point of view of the user). If we start another thread or
process to do the scanning, we would be able to kill it immediately
whenever the frustrated user hits the cancel button.
Every solution has its drawbacks (and sometimes some advantages).

Mathias

PS: sorry for being OT.

 

[Christopher Benson-Manica]

(Moving to comp.programming.threads - if you don't read that group,
come back to clc++...)

Because I don't research OS kernels, embedded software, or other primitive
spaces that need threads to get anything done, I have never seen a situation
improved by a thread. Throwing a thread at a simple problem is worse than
'goto'. If a button starts a process that takes too long, the process itself
needs a better architecture.

Well, my specific intention for this thread is for it to take care of
caching the contents of a file in memory as my program is starting.
The main thread can't wait that long, because the program is a Windows
service and the service manager isn't willing to wait forever for my
program to start. I know I could do this other ways, but is there
really a big disadvantage to spawning a thread to handle the caching
of this file?

 

[Phlip]

Hmmm... what exactly would "immoral" C++ code be?

Dive in: http://sf.net

 

[Phlip]

B would require some documentation letting users know how it was
intended to be used, however. Maybe the way to do it (what I really
want to do) would be to wrap the class that destroys itself in a class
that doesn't care, making it impossible to misuse it?

Ding! Objects should be easy to use right and hard to use wrong.

About "documentation", try some test cases that show each object's normal
lifecycle. Class users (including yourself) will be unlikely to deviate.

Again, I don't mind nitpicks, especially since I know I'm living in a
bubble of sorts at my present place of employement...

How DARE you write 'cout' inside a non-IO class?!!

 

[Phlip]

Of course its necessary to obtain the linked list of files (in the same
manner). And we silently assume, that "is it infected" will never block
(block from the point of view of the user). If we start another thread or
process to do the scanning, we would be able to kill it immediately
whenever the frustrated user hits the cancel button.
Every solution has its drawbacks (and sometimes some advantages).

"Is it infected", in turn, runs a list of potential virii, so we have
another spot to decouple.

From here, event-driven architectures are overwhelmingly easy to switch to
use threads, because the places between the events, for other activities,
are also good places for semaphores. But this doesn't work the opposite way.
Poorly architected systems might be hard to convert from threads to events.
So, either way, focus on good architecture.

Well, my specific intention for this thread is for it to take care of
caching the contents of a file in memory as my program is starting.
The main thread can't wait that long, because the program is a Windows
service and the service manager isn't willing to wait forever for my
program to start. I know I could do this other ways, but is there
really a big disadvantage to spawning a thread to handle the caching
of this file?

Short term, no. Removing this thread won't force the architecture to
improve.

Long term, use ReadFileEx() in asynchronous mode. If you need notification
that the file arrived, you can multiplex the file event semaphore in with
the window messages using MsgWaitForMultipleObjects().

Or start a window timer that wakes up and reads a small chunk of the file.
The file system will buffer the real file asynchronously, behind these
reads, meaning they shouldn't block the GUI.

Notice the tutorials tend to teach threading _before_ they teach these
high-powered methods to cleanly avoid threads...

 

[Christopher Benson-Manica]

Long term, use ReadFileEx() in asynchronous mode. If you need notification
that the file arrived, you can multiplex the file event semaphore in with
the window messages using MsgWaitForMultipleObjects().

Well, I guess I should have chosen my words more carefully - the file
has the contents of a list of data structures, each of which needs to
be parsed before the program can begin its real work.

Notice the tutorials tend to teach threading _before_ they teach these
high-powered methods to cleanly avoid threads...

Yes, I do notice that, and I'll remember them (hopefully) next time I
do GUI work

 

[Christopher Benson-Manica]

How DARE you write 'cout' inside a non-IO class?!!

Well, neither I nor my coworkers much care, but presumably there are
places and bosses where that is the standard, so it's worth at least
being thus forewarned...