What happens to unique_ptr -pointer in this case

[JiiPee]

Just cannot be sure what happens if I do the following:

class A
{
public:
void foo() {
delete this;
}
};

int main()
{
std::unique_ptr<A> b;
b.foo();
// (*) here: what is the state of b?
}

How does b know that its object is deleted? Can i use b normally after
(*) or is it in undefined state?
If I want to delete the object inside foo(), how should i do it so that
b is in good state, or is it even possible?
Is this a wrong way to use unique_ptr (so we should not delete object
inside its member variable)?

[JiiPee]

Sorry, should be like this in main:
std::unique_ptr<A> b = std::make_unique<A>();
b->foo();

[Chris Vine]

b.foo() won't compile, and presumably you meant b->foo(). If so, yes,
you have it completely wrong. First you have not actually allocated
an A object, so b->foo will dereference a null pointer and the program
will terminate. Secondly even if you had allocated an A object to be
managed by b, your code would cause a double delete, memory corruption
and woe, because b->foo() would delete the A object, and b would try
to do the same when it goes out of scope.

You need to look up some documentation on unique_ptr. They are very
simple things - it should take you about 10 minutes to work it out.
You could do what you want by releasing the unique_ptr when you call
A::foo() (see std::unique_ptr::release()), but having an object know
what object manages it defeats the purpose of using unique_ptr in the
first place. Just leave it to the unique_ptr to deal with
deallocation, or adopt a different design.

[JiiPee]

On 24/11/2016 00:56, Chris Vine wrote:
> memory corruption
> and woe, because b->foo() would delete the A object, and b would try
> to do the same when it goes out of scope.

Thats what I also thought.

The reason I ask this is becouse with Microsoft MFC its quite common
practice for people to do:

delete this;

When the window-objects window gets destroyed:

void CMyDialog::OnNcDestroy()
{
CDialog::OnNcDestroy();
delete this;
}

So, is this a wrong approach? Many do this though....and I think also
MFC does that something inside the library?

(In this example we know that CMyDialog is created dynamically with new)

[JiiPee]

On 24/11/2016 00:56, Chris Vine wrote:

> b.foo() won't compile, and presumably you meant b->foo(). If so, yes,
> you have it completely wrong. First you have not actually allocated
> an A object, so b->foo will dereference a null pointer and the program
> will terminate.

Yes, there was a mistake.. see my other post

[JiiPee]

On 24/11/2016 00:56, Chris Vine wrote:

> Just leave it to the unique_ptr to deal with
> deallocation, or adopt a different design.

So this means that "delete this;" is not a good practise, right? Because
it might lead to confusion.

[Jerry Stuckle]

But MFC doesn't use unique_ptr, so someone has to delete it or you have
a memory leak. And after the return from OnNcDestroy() the object
should never be referenced again (and isn't in MFC).

--
==================
Remove the "x" from my email address
Jerry Stuckle
jstu...@attglobal.net
==================

[Jerry Stuckle]

It all depends on the circumstances. If you do use it, ensure to
document it.

[Alf P. Steinbach]

When `std::unique_ptr` destructor attempts to destroy the object it's
already destroyed, and you have Undefined Behavior.

The proper way for the object to self-destruct is to do that via the
single owner, here the smart pointer.

Which means the object needs knowledge of its owner, or be provided with
a self-destruct operation that does.


Cheers & hth.,

- Alf

[Paavo Helde]

On 24.11.2016 3:19, JiiPee wrote:
> On 24/11/2016 00:56, Chris Vine wrote:
>> memory corruption
>> and woe, because b->foo() would delete the A object, and b would try
>> to do the same when it goes out of scope.
>
>
> Thats what I also thought.
>
> The reason I ask this is becouse with Microsoft MFC its quite common
> practice for people to do:
>
> delete this;
>
> When the window-objects window gets destroyed:
>
> void CMyDialog::OnNcDestroy()
> {
> CDialog::OnNcDestroy();
> delete this;

After this, there must not be another delete called for this object.
This means that either the object was not managed by smart pointers at
all, or that OnNcDestroy() was called from the destructor of the last
smart pointer.

In case of MFC it is the former, the objects are managed by raw pointers
by using strict ownership conventions. This is possible because MFC is
single-threaded.

Cheers
Paavo

[Paavo Helde]

'Delete this;' is rarely a good practice. Since C++11 one has several
standardized smartpointers which should be used instead.

In general, a naked 'delete' should not appear in application level code
at all. If not for other reasons, then just because it is inherently
exception-unsafe.

With legacy frameworks like MFC one needs to use their conventions of
course, and if this requires writing 'delete this;' then so be it.

Cheers
Paavo

[Chris Vine]

'delete this' is OK in cases where an object is entirely self-owning.
Non-modal GUI dialogs in an asynchronous event loop are one of the few
examples - their lifetime is ended by the program user clicking on a
close button or the like (which are events which the object itself
receives and handles in many GUI systems), and bears no relation to
anything else in the program (apart from program termination). If the
dialog receives the click notification, it might as well kill itself
with 'delete this' rather than delegate that to some firing squad it
specially sets up for the purpose. You have to obey the usual
conventions - once 'delete this' has been called, no non-static member
data of the object may be accessed and no non-static methods of the
object may be called. The only thing the method can sensibly do is
return, which yours did.

I have code which does that with no embarassment. It's all a question
of clear ownership. If you use a unique_ptr to manage an object
allocated on free store, you are declaring that the unique_ptr (or some
other one to which it is moved) is responsible for managing lifetime.
In that case you should let the unique_ptr do its thing. For most
usages unique_ptr is ideal. If you need shared ownership then use
shared_ptr. Only in the case of self-ownership (usually, as I say, in
event driven programming) should you consider 'delete this'.

Chris

[Öö Tiib]

On Thursday, 24 November 2016 09:31:45 UTC+2, Paavo Helde wrote:
> On 24.11.2016 3:26, JiiPee wrote:
> > On 24/11/2016 00:56, Chris Vine wrote:
> >> Just leave it to the unique_ptr to deal with
> >> deallocation, or adopt a different design.
> >
> >
> > So this means that "delete this;" is not a good practise, right? Because
> > it might lead to confusion.
> >
>
> 'Delete this;' is rarely a good practice. Since C++11 one has several
> standardized smartpointers which should be used instead.
>
> In general, a naked 'delete' should not appear in application level code
> at all. If not for other reasons, then just because it is inherently
> exception-unsafe.

Interesting fact that one usage of 'delete this' that I have seen was for
to achieve strong exception safety (erase-commit/rollback).

[JiiPee]

On 24/11/2016 03:57, Jerry Stuckle wrote:
> But MFC doesn't use unique_ptr, so someone has to delete it or you have
> a memory leak. And after the return from OnNcDestroy() the object
> should never be referenced again (and isn't in MFC).

but even if it is not a smart pointer, the user must still *know* that
the function will destroy it. Is there a risk here and confusion that if
the user forgots that its self deleted? How does the user remember that
in this occasion the object will be self-destroyed?

[JiiPee]

On 24/11/2016 07:31, Paavo Helde wrote:
> With legacy frameworks like MFC one needs to use their conventions of
> course, and if this requires writing 'delete this;' then so be it.

You would not mix the modern C++ with MFC?

[JiiPee]

Ok, this was pretty clear explanation. So in this occasion its ok
because MFC uses that convention.

[Jerry Stuckle]

It's called documentation. And a user can forget anything is deleted.
It has nothing to do with whether it is self-deleted or not.

You must also remember that this is in response to an asynchronous call
from within Windows, not from the application itself. The object cannot
be deleted before this because it must be there to handle the message.
And the object is already considered "gone" to the rest of the application.

[Paavo Helde]

On 24.11.2016 14:35, JiiPee wrote:
> On 24/11/2016 03:57, Jerry Stuckle wrote:
>> But MFC doesn't use unique_ptr, so someone has to delete it or you have
>> a memory leak. And after the return from OnNcDestroy() the object
>> should never be referenced again (and isn't in MFC).
>
>
> but even if it is not a smart pointer, the user must still *know* that
> the function will destroy it. Is there a risk here and confusion that if

Confusion is the middle name of MFC! :-)

> the user forgots that its self deleted? How does the user remember that
> in this occasion the object will be self-destroyed?

What do you mean by "user"? If you mean the programmer, then he is
supposed to follow the documentation. If it is the calling code, then
this calling code is MFC which *assumes* that dialog will be dead after
OnNcDestroy(). This function should not be called explicitly from
elsewhere than MFC.

Besides, MSDN documentation says that 'delete this;' should be put into
virtual PostNcDestroy() override which is called from the default
implementation of OnNcDestroy(), so whoever placed it directly in
OnNcDestroy() already violated the conventions and caused confusion indeed.

"https://msdn.microsoft.com/library/49a832ee-bc34-4126-88b3-bc1d9974f6c4.aspx#cwnd__onncdestroy"

[Paavo Helde]

Sure one can mix code, but one cannot redesign MFC to use another
ownership model for its objects.

[JiiPee]

On 24/11/2016 16:03, Paavo Helde wrote:
> On 24.11.2016 14:35, JiiPee wrote:
>> On 24/11/2016 03:57, Jerry Stuckle wrote:
>>> But MFC doesn't use unique_ptr, so someone has to delete it or you have
>>> a memory leak. And after the return from OnNcDestroy() the object
>>> should never be referenced again (and isn't in MFC).
>>
>>
>> but even if it is not a smart pointer, the user must still *know* that
>> the function will destroy it. Is there a risk here and confusion that if
>
> Confusion is the middle name of MFC! :-)

hehe. Well, I still like it (even if its not perfect/pretty). Remember,
i have lived my whole programming life with it!! And I dont like
divorces, hehe. People take divorces too easily.. I am a faithfull man :).

>
>> the user forgots that its self deleted? How does the user remember that
>> in this occasion the object will be self-destroyed?
>
> What do you mean by "user"?

the one who uses that Dialog. The programmer who writes code to use that
dialog.

> If you mean the programmer, then he is supposed to follow the
> documentation. If it is the calling code, then this calling code is
> MFC which *assumes* that dialog will be dead after OnNcDestroy().

but the question is that do we delete the dialog object inside the
dialog class (at OnNcDestroy call) or from the class where it is
created. But seems like Microsoft supports this... below:

> This function should not be called explicitly from elsewhere than MFC.
>
> Besides, MSDN documentation says that 'delete this;' should be put
> into virtual PostNcDestroy() override which is called from the default
> implementation of OnNcDestroy(), so whoever placed it directly in
> OnNcDestroy()

Good catch. Oh, now i get it. I was capturing the message directly, but
I should use the virtual call. Ok, but online many people seem to use
directly OnNcDestroy(); message function.

> already violated the conventions and caused confusion indeed.
>
> "https://msdn.microsoft.com/library/49a832ee-bc34-4126-88b3-bc1d9974f6c4.aspx#cwnd__onncdestroy"
>

but the confusion I mentioned is more like with the delete this;

>
>
>