5

As far as I see the code below is correct:

#include <iostream>
#include <functional>

struct A
{
    std::function<void ()> func;
    
    int value = 5;
    
    ~A() { func(); }
};

int main()
{
    A* p_a = new A();
    p_a->func = [&p_a]() { std::cout << p_a->value; };
    delete p_a;
    return 0;
}

but the following code is not and results in a segmentation fault:

#include <memory>

int main()
{
    std::unique_ptr<A> p_a = std::make_unique<A>();
    p_a->func = [&p_a]() { std::cout << p_a->value; };
    p_a = {};
    return 0;
}

std::unique_ptr clears its internal pointer first and then deletes the object and thus prevents access to the object being destroyed.

What is it for? What is the logic behind this?

EDIT1:

Marked the question is a duplicate. I would prefer std::uniqu_ptr to keep old object if the deleter throws an exception (if I did not misunderstand something).

Dmitriano
  • 1,878
  • 13
  • 29
  • [This](https://stackoverflow.com/questions/67002935/clarification-about-object-lifetime-in-destructor) seems somewhat related. – super Sep 04 '21 at 15:24
  • 1
    On another note, just because the first example doesn't generate a segmentation fault doesn't mean it's valid. – super Sep 04 '21 at 15:28
  • The second example is calling the function with a reference to the smart pointer. By the time the destructor is called, the smart pointer has already been reset to nullptr. Dereferencing a nullptr is undefined behavior. One fix would be `p_a->func = [&p_a]() { if (p_a) std::cout << p_a->value << "\n"; };` – Eljay Sep 04 '21 at 15:29
  • @super `A::func` is still alive when the destructor starts execution. – Dmitriano Sep 04 '21 at 15:30
  • @Eljay That's what the OP says. I guess the question is - at least how I understand it - why the order is not: First delete the currently managed object, and after that replace it with the `nullptr`. – t.niese Sep 04 '21 at 15:30
  • When you call `p_a = {};` (or equivalently or more clearly, `p_a.reset();`), you tell `p_a` that it should no longer own any object. Why *should* you be able to access the old object through `p_a` at any point after that operation is invoked? You said the internal pointer should be null, and it became null. The program did what you told it to. – JaMiT Sep 04 '21 at 15:33
  • @JaMiT The pointer is already `nullptr` but the object is still alive. Imagine you have a cat that is still alive but you can't access it. – Dmitriano Sep 04 '21 at 15:37
  • @Dmitriano That's a bad analogy. Unlike the cat, the object is not yours. The object belongs to the smart pointer. You have no rights to that object other than what the `unique_ptr` API grants you. – JaMiT Sep 04 '21 at 15:41
  • @JaMiT So another analogy, imaging a bad guy `unique_ptr` that prevents you from accessing your cat. – Dmitriano Sep 04 '21 at 15:43
  • @Dmitriano If the `unique_ptr` can prevent you from accessing the cat, then the cat belongs to the unique_ptr. It is not **your** cat! In fact, you've become the bad guy because you want to access the `unique_ptr`'s cat without permission. – JaMiT Sep 04 '21 at 16:24

2 Answers2

2

This implementation is done for preventing recursion in std::unique_ptr::reset().

When you take std::unique_ptr by reference, you don't own std::unique_ptr and don't control the life-time of the referenced object. The correct code must always check for nullptr the raw pointer.

int main()
{
    std::unique_ptr<A> p_a = std::make_unique<A>();
    p_a->func = [&p_a]() { if (p_a) std::cout << p_a->value; };
    p_a = {};
    return 0;
}
273K
  • 29,503
  • 10
  • 41
  • 64
  • Ah ok, so it prevents a recursion if user would do a `p_a->func = [&p_a]() { p_a = /* something else */ };`, which would occure when setting the internal pointer and destructing the object would be done the other way round. – t.niese Sep 04 '21 at 15:35
  • Yes, it prevents from any implicit or explicit reset called from the destructor of the referenced object. – 273K Sep 04 '21 at 15:38
  • Yes, probably it prevents a recursion when `reset` throws an exception (from the deleter) and then `unique_ptr` destructor is called. – Dmitriano Sep 05 '21 at 14:13
1

This is just a comment, but too long to fit into the comment section. So I post it as an answer.

First lets look at the standard, it says

u can, upon request, transfer ownership to another unique pointer u2. Upon completion of such a transfer, the following postconditions hold:
(4.1) — u2.p is equal to the pre-transfer u.p,
(4.2) — u.p is equal to nullptr, and
(4.3) — if the pre-transfer u.d maintained state, such state has been transferred to u2.d.
As in the case of a reset, u2 must properly dispose of its pre-transfer owned object via the pre-transfer associated deleter before the ownership transfer is considered complete.

The standard only specified the behavior after the reset, the exact behavior during reset is implementation-dependent.

Then let's look at the GNU implementation:

void reset(pointer __p) noexcept
{
  const pointer __old_p = _M_ptr();
  _M_ptr() = __p;
  if (__old_p)
    _M_deleter()(__old_p);
}
~unique_ptr() noexcept
{
  static_assert(...);
  auto& __ptr = _M_t._M_ptr();
  if (__ptr != nullptr)
    get_deleter()(std::move(__ptr));
  __ptr = pointer();
}

Which looks quite odd because the exact order are different in reset and dtor. It doesn't make sense to me, until I read the MSVC implementation:

void reset(pointer _Ptr = nullptr) noexcept {
    pointer _Old = _STD exchange(_Mypair._Myval2, _Ptr);
    if (_Old) {
        _Mypair._Get_first()(_Old);
    }
}

When reset, of course we need to swap current pointer with the argument. Also, don't forget to call the deleter. Simple job.

~unique_ptr() noexcept {
    if (_Mypair._Myval2) {
        _Mypair._Get_first()(_Mypair._Myval2);
    }
}

In destructor, Microsoft doesn't bother set the pointer to nullptr, they just call the deleter. Simple.

If you really want to set the pointer to nullptr, I bet you will set it after you call the deleter.


Conclusion:

I don't think the STL authors really want to "prevent" you doing something. They just simply write the most straight-forward code to implement the requirement.

JaMiT
  • 14,422
  • 4
  • 15
  • 31
yyyy
  • 593
  • 2
  • 10
  • `reset()` and `~unique_ptr()` are different in GNU standard library because destructors can't be implicitly called recursively, `reset` can. – 273K Sep 04 '21 at 17:38
  • @S.M. `class A { unique_ptr a; A() { a.reset(this); } }` – yyyy Sep 04 '21 at 17:46
  • And? The code is wrong, circular dependency. The destructor of A called twice, the same issue as delete a twice. – 273K Sep 04 '21 at 18:00
  • *"This is just a comment, but too long to fit into the comment section."* -- if a comment is too long to fit in the comment section, then it does not fit SO's guidelines for comments, so should not be posted. However, in this case, you might be wrong about it being "just" a comment. It is at least pretty close to being an answer. – JaMiT Sep 04 '21 at 20:00