10

Calling void reset( pointer ptr = pointer() ) noexcept; invokes the following actions

Given current_ptr, the pointer that was managed by *this, performs the following actions, in this order:

  1. Saves a copy of the current pointer old_ptr = current_ptr
  2. Overwrites the current pointer with the argument current_ptr = ptr
  3. If the old pointer was non-empty, deletes the previously managed object if(old_ptr) get_deleter()(old_ptr).

cppreference

What is the reason for this particular order? Why not just do 3) and then 2)? In this question std::unique_ptr::reset checks for managed pointer nullity? the first answer quotes the Standard

[…] [ Note: The order of these operations is significant because the call to get_deleter() may destroy *this. —end note ]

Is this the only reason? How could get_deleter() destroy the unique_ptr (*this)?

Ruperrrt
  • 489
  • 2
  • 13
  • 1
    Say this `unique_ptr` points to some object `A`, and is in turn a data member of some object `B`. It's possible that a destructor of `A` deletes `B`, which in turn destroys the pointer. You want it to be null at that time, or else you'll end up with double destruction. – Igor Tandetnik Sep 01 '21 at 01:04
  • 1
    Also that order is safer if ever the destructor throw (bad practice) – Phil1970 Sep 01 '21 at 01:35
  • 1
    @Phil1970 destructor can't throw. – prehistoricpenguin Sep 01 '21 at 01:58
  • 4
    @prehistoricpenguin it's a bad idea and the language defaults to noexcept destructors, but it can be done. – Raymond Chen Sep 01 '21 at 03:30
  • @IgorTandetnik Could you specify an example from practice where the destructor of `A` would delete `B`? I can't imagine one myself. – Ruperrrt Sep 03 '21 at 14:52

2 Answers2

3

It's often useful when analysing a prescribed order of steps, to consider which ones could throw and and what state that would leave everything in - with the intent that we should never end up in an unrecoverable situation.

Note that from the docs here that:

Unlike std::shared_ptr, std::unique_ptr may manage an object through any custom handle type that satisfies NullablePointer. This allows, for example, managing objects located in shared memory, by supplying a Deleter that defines typedef boost::offset_ptr pointer; or another fancy pointer.

So, in the current order:

  1. Saves a copy of the current pointer old_ptr = current_ptr

    if the copy constructor of fancy pointer throws, unique_ptr still owns the original object and new object is un-owned: OK

  2. Overwrites the current pointer with the argument current_ptr = ptr

    if the copy assignment of fancy pointer throws, unique_ptr still owns the original object and new object is un-owned: OK

    (this assumes the fancy pointer's copy assignment operator meets the usual exception-safety guarantee, but there's not much unique_ptr can do without that)

  3. If the old pointer was non-empty, deletes the previously managed object if(old_ptr) get_deleter()(old_ptr)

    at this stage unique_ptr owns the new object and it is safe to delete the old one.

In other words, the two possible outcomes are:

std::unique_ptr<T, FancyDeleter> p = original_value();
try {
  auto tmp = new_contents();
  p.reset(tmp);
  // success
}
catch (...) {
  // p is unchanged, and I'm responsible for cleaning up tmp
}

In your proposed order:

  1. If the original pointer is non-empty, delete it

    at this stage unique_ptr is invalid: it has committed the irreversible change (deletion) and there is no way to recover a good state if the next step fails

  2. Overwrite the current pointer with the argument current_ptr = ptr

    if the copy assignment of fancy pointer throws, our unique_ptr is rendered unusable: the stored pointer is indeterminate and we can't recover the old one

In other words, the unrecoverable situation I'm talking about is shown here:

std::unique_ptr<T, FancyDeleter> p = original_value();
try {
  auto tmp = new_contents();
  p.reset(tmp);
  // success
}
catch (...) {
  // I can clean up tmp, but can't do anything to fix p
}

After that exception, p can't even be safely destroyed, because the result of calling the deleter on its internal pointer might be a double-free.


NB. The deleter itself is not permitted to throw, so we don't have to worry about that.

The note saying

... the call to get_­deleter() might destroy *this.

sounds wrong, but the call get_­deleter()(old_­p) really might ... if *old_p is an object containing a unique_ptr to itself. The deleter call has to go last in that case, because there is literally nothing you can safely do to the unique_ptr instance after it.

Although this corner case is a solid reason for putting the deleter call at the end, I feel like the strong exception safety argument is perhaps less contrived (although whether objects with unique pointers to themselves are more or less common than fancy pointers with throwing assignment is anyone's guess).

Useless
  • 64,155
  • 6
  • 88
  • 132
1

Similarly to std::enable_shared_from_this you can create objects that use std::unique_ptr to manage their own lifetime, without the overhead of std::shared_ptr whenever a std::unique_ptr is all you need. In the example below we create a "fire and forget" Task that self-destructs after doing its work. Motivation can be found here.

#include <cstdio>
#include <future>
#include <memory>
#include <thread>

using namespace std::chrono_literals;

class Task
{
  public:
    Task(Task&&) = delete;
    Task& operator=(Task&&) = delete;
    Task(const Task&) = delete;
    Task& operator=(const Task&) = delete;
    ~Task() { std::printf("Destroyed.\n"); }

    static Task* CreateTask()
    {
        // Can't use std::make_unique because the constructor is private.
        std::unique_ptr<Task> task{new Task{}};
        Task* const result{task.get()};
        result->AcceptOwnershipOfSelf(std::move(task));
        return result;
    }

    void Run()
    {
        // Do work ...

        // Work is done. Self-destruct.
        self_.reset();
    }

  private:
    // Constructor needs to be private: Task must be created via `CreateTask`.
    Task() = default;
    void AcceptOwnershipOfSelf(std::unique_ptr<Task> self) { self_ = std::move(self); }

    std::unique_ptr<Task> self_;
};

int main()
{
    Task* const task{Task::CreateTask()};
    std::ignore = std::async(&Task::Run, task);
    std::this_thread::sleep_for(1s);
}

godbolt. Note that "Destroyed." is only printed once.

Why not just do 3) and then 2)?

If self_.reset(); were to delete the previously managed pointer before zeroing current_ptr, self_ would still point to *this in ~Task - leading to an infinite loop. We can see that by replacing self_.reset(); with self_->~Task();.

Of course we could replace self_.reset(); with the following two-liner:

        std::unique_ptr<Task> tmp{std::move(self_)};
        tmp.reset();

I don't know whether the committee decided to specify reset as they did because of this use case or not.

Notes:

Tim Rakowski
  • 97
  • 2
  • 7
  • Is the use of `std::unique_ptr` in this technique supposed to be an improvement over `new`/`delete`? In what way if it is? – techolic Sep 07 '21 at 20:10
  • I didn't say that. It's hard to find benefits: The usual answer would be that `std::unique_ptr` avoids leaks, especially when faced with exceptions. But using `new` in `CreateTask` can't lead to leaks there because none of the following operations can throw. And later there is no scope at whose end the `std::unique_ptr` would take care of deleting the `Task`. So I think that there would only be a benefit in codebases where raw `new`/`delete` is simply forbidden. Often https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r11-avoid-calling-new-and-delete-explicitly has to be followed. – Tim Rakowski Sep 07 '21 at 21:32
  • While that sample is contrived, avoiding infinite loop if every there is a cycle `A --> B --> C --> D --> A` would be a good reason as resetting any item of the loop would destroy them all. – Phil1970 Sep 07 '21 at 23:48