Unique ptr move ownership to containing object's method

I'd like to move unique_ptr to its object's method:

class Foo {
    void method(std::unique_ptr<Foo>&& self) {
        // this method now owns self
    }
}

auto foo_p = std::make_unique<Foo>();
foo_p->method(std::move(foo_p));

This compiles, but I don't know if it is not Undefined behavior. Since I moved from the object when also calling a method on it.

Is it UB?

If it is, I could probably fix it with:

auto raw_foo_p = foo_p.get();
raw_foo_p->method(std::move(foo_p))

right?

(Optional Motivation:)

Pass the object around to extend its lifetime. It would live in a lambda until the lambda would be called asynchronously. (boost::asio) Please, see Server::accept first and then Session::start.

You can see the original implementation used shared_ptr, but I don't see why would that be justified, since I only need one owner of my Session object.

Shared_ptr makes code more complex and it was hard for me to understand, when not familiar with shared_ptr.

#include <iostream>
#include <memory>
#include <utility>

#include <boost/asio.hpp>

using namespace boost::system;
using namespace boost::asio;
using boost::asio::ip::tcp;


class Session /*: public std::enable_shared_from_this<Session>*/ {
public:
    Session(tcp::socket socket);

    void start(std::unique_ptr<Session>&& self);
private:
    tcp::socket socket_;
    std::string data_;
};

Session::Session(tcp::socket socket) : socket_(std::move(socket))
{}

void Session::start(std::unique_ptr<Session>&& self)
{
    // original code, replaced with unique_ptr
    // auto self = shared_from_this();

    socket_.async_read_some(buffer(data_), [this/*, self*/, self(std::move(self))]  (error_code errorCode, size_t) mutable {
        if (!errorCode) {
            std::cout << "received: " << data_ << std::endl;
            start(std::move(self));
        }

        // if error code, this object gets automatically deleted as `self` enters end of the block
    });
}


class Server {
public:
    Server(io_context& context);
private:
    tcp::acceptor acceptor_;

    void accept();
};


Server::Server(io_context& context) : acceptor_(context, tcp::endpoint(tcp::v4(), 8888))
{
    accept();
}

void Server::accept()
{
    acceptor_.async_accept([this](error_code errorCode, tcp::socket socket) {
        if (!errorCode) {
            // original code, replaced with unique_ptr
            // std::make_shared<Session>(std::move(socket))->start();

            auto session_ptr = std::make_unique<Session>(std::move(socket));
            session_ptr->start(std::move(session_ptr));
        }
        accept();
    });
}

int main()
{
    boost::asio::io_context context;
    Server server(context);
    context.run();
    return 0;
}

compiles with: g++ main.cpp -std=c++17 -lpthread -lboost_system

Solution

For your first code block:

std::unique_ptr<Foo>&& self is a reference and assigning it an argument std::move(foo_p), where foo_p is a named std::unique_ptr<Foo> will only bind the reference self to foo_p, meaning that self will refer to foo_p in the calling scope.

It does not create any new std::unique_ptr<Foo> to which the ownership of the managed Foo object may be transferred. No move construction or assignment happens and the Foo object is still destroyed with the destruction of foo_p in the calling scope.

Therefore there is no risk of undefined behavior in this function call itself, although you could use the reference self in a way that could cause undefined behavior in the body.

Maybe you intended to have self be a std::unique_ptr<Foo> instead of std::unique_ptr<Foo>&&. In that case self would not be a reference, but an actual object to which ownership of the managed Foo would be transferred via move construction if called with std::move(p_foo) and which would be destroyed after the function call in foo_p->method(std::move(foo_p)) together with the managed Foo.

Whether this alternative variant is in itself potentially undefined behavior depends on the C++ standard version in use.

Before C++17 the compiler was allowed to choose to evaluate the call's arguments (and the associated move construction of the parameter) before evaluating foo_p->method. This would mean, that foo_p could have already moved from when foo_p->method is evaluated, causing undefined behavior. This could be fixed similarly to how you propose to do it.

Since C++17 it is guaranteed that the postfix-expression (here foo_p->method) is evaluated before any of the arguments of the call are and therefore the call itself would not be a problem. (Still the body could cause other issues.)

In detail for the latter case:

foo_p->method is interpreted as (foo_p->operator->())->method, because std::unique_ptr offers this operator->(). (foo_p->operator->()) will resolve to a pointer to the Foo object managed by the std::unique_ptr. The last ->method resolves to a member function method of that object. In C++17 this evaluation happens before any evaluation of the arguments to method and is therefore valid, because no move from foo_p has happened yet.

Then the evaluation order of the arguments is by design unspecified. So probably A) the unique_ptr foo_p could get moved from before this as an argument would be initialized. And B) it will get moved from by the time method runs and uses the initialized this.

But A) is not a problem, since § 8.2.2:4, as expected:

If the function is a non-static member function, the this parameter of the function shall be initialized with a pointer to the object of the call,

(And we know this object was resolved before any argument was evaluated.)

And B) won't matter as: (another question)

the C++11 specification guarantees that transferring ownership of an object from one unique_ptr to another unique_ptr does not change the location of the object itself

For your second block:

self(std::move(self)) creates a lambda capture of type std::unique_ptr<Session> (not a reference) initialized with the reference self, which is referring to session_ptr in the lambda in accept. Via move-construction ownership of the Session object is transferred from session_ptr to the lambda's member.

The lambda is then passed to async_read_some, which will (because the lambda is not passed as non-const lvalue reference) move the lambda into internal storage, so that it can be called asynchronously later. With this move, the ownership of the Session object transfers to the boost::asio internals as well.

async_read_some returns immediately and so all local variables of start and the lambda in accept are destroyed. However the ownership of Session was already transferred and so there is no undefined behavior because of lifetime issues here.

Asynchronously the lambda's copy will be called, which may again call start, in which case the ownership of Session will be transferred to another lambda's member and the lambda with the Session ownership will again be moved to internal boost::asio storage. After the asynchronous call of the lambda, it will be destroyed by boost::asio. However at this point, again, ownership has already transferred.

The Session object is finally destroyed, when if(!errorCode) fails and the lambda with the owning std::unique_ptr<Session> is destroyed by boost::asio after its call.

Therefore I see no problem with this approach with regards to undefined behavior relating to Session's lifetime. If you are using C++17 then it would also be fine to drop the && in the std::unique_ptr<Session>&& self parameter.