Re: Hello everybody, I have something to share about the shared_ptr class

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2020-07-31 17:40:14

On 2020-07-31 19:51, 逸霖 杨 via Boost wrote:
> Hi Andrey,
> 
> Sorry for Top-Post. But I searched and still don’t know what is Top-Post, pardon me, and please explain a little.

The page I linked before contained a link to Wikipedia that explains it. 
Here is that link:

https://en.wikipedia.org/wiki/Posting_style#Top-posting

> Reply about :
> 
> After the destructor call, the object no longer exists, regardless of
> whether the memory it used to be placed in is freed or not.
> =====================================
> This is true as a concept, but when we overload delete operator for global or for just one class,
> we write “free(ptr)” inside that overloaded function (at least MS’ implementation of C++ works like this).

Right, that's how storage allocation functions work. By the time they 
are invoked, the object is either not yet created or is already destroyed.

> ==================================================================
> Case 1:
> 
>     struct A
>     {
>       unsigned int x = 0u;
> 
>       ~A() { x = 0xBAADF00D; }
>     };
> 
>     rcgc_shared_ptr< A > p1;
>     {
>       rcgc_shared_ptr< A > p2(new A());
>       p1 = p2;
>       std::cout << p1->x << std::endl;
>     }
>     std::cout << p1->x << std::endl; // (1)
> 
> The above is a bug because (1) accesses the object A after its
> destruction. If A::x was, say a unique_ptr, it would be null or pointing
> to already freed memory.
> ======================
> Why would we assign x with a useful value in destructors ?

Does it matter? The example shows that you're accessing a destroyed 
object, which you confirmed in the output below.

> Unlike the predicted behavior on my side,
> this Case1 I run in main function, results in
> 0
> baadf00d
> (<<std::hex<< is added before to show hex value instead of dec value)

> Case 2 (which follows from Case 1)
> 
>     struct B;
> 
>     struct A
>     {
>       rcgc_shared_ptr< B > m_b;
>       std::string m_x;
> 
>       ~A() { std::cout << m_b->m_x << std::endl; }
>     };
> 
>     struct B
>     {
>       rcgc_shared_ptr< A > m_a;
>       std::string m_x;
> 
>       ~B() { std::cout << m_a->m_x << std::endl; }
>     };
> 
>     rcgc_shared_ptr< A > p(new A());
>     p->m_b.reset(new B());
>     p->m_b->m_a = p;
> 
> When p gets destroyed, it will destroy the object A, even though it is
> still referenced by B::m_a. When A::m_b is destroyed, it will call ~B,
> which will access A::m_x, which is already destroyed.
> ======================================
> I’m sorry that rcgc_shared_ptr does not provide reset function.
> I didn’t think of this yet, but will do.

The reset function has the same semantics as for shared_ptr. It doesn't 
change my point though.

>> If you have Visual Studio and run the code which detects memory leak with
>> #ifdef _WIN32
>>       _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
>> #endif
>>
>> You will see no memory leak at all.
> 
> This check only shows that no memory is leaked. The problem here is not
> a memory leak but incorrect memory access (potential use-after-destory
> and use-after-free).
> ==========================================
> The thing is in reality, calling destructor is not same as freeing memory.

True.

> Memory is still containing valid data if no one messes with it.

Not true.

In general, accessing memory after object destruction is undefined 
behavior. Whether that memory contents still contain the data that was 
last present in the destroyed object is not guaranteed.

> And if
> the free(ptr) function is not called, no one can/should mess with it.
> The only thing would possible is calling the destructor for multiple times
> because the rcgc_shared_ptrs requests this action. But use-after-destroy
> can be still avoided.
> 
> To avoid use-after-destroy, just keep the good manner like
> 
> ~A(){
>    If(this->p!=nullptr)
>    {
> delete this->p;
> this->p = nullptr;
>    }
> }
> And everything will be safe.

No, it won't be safe. As an example of a real practical problem with 
this code is that compilers are allowed to remove the assignment of 
nullptr to the class member because the object is destroyed and no valid 
code will read that member after destruction. Basically, that assignment 
is dead code.

https://gcc.godbolt.org/z/rnncjn

> And also, no one should do important work
> inside of destructors besides freeing and avoid double freeing, don’t they?

Does it matter what people do in destructors? Destructors are not 
supposed to be called on a non-existing object. (BTW yes, people do 
non-trivial things in destructors besides releasing resources.)

You seem to be missing the fact that C++ doesn't work with just bytes in 
memory and has an actual object model.

> BTW, is it correct that intuitive_ptr is another kind of COM IUnknown ?

I'm not sure I understand this question. Yes, you can use intrusive_ptr 
to reference IUnknown and derived objects.

> What would you do if a reference is pointed to the object itself?

I'm not sure what use case you mean. In general, I don't have to do 
anything special about cycle resolution with intrusive_ptrs because 
either cycles are not possible in that code or they resolve 
automatically (e.g. there is a destroy() method that explicitly resets 
all pointers stored in the object before returning).