C++17 and asynchronous member functions calling with move capture lambda expression

Question

In the other question I've asked, I've learned some of evaluation orders are well defined since C++17. postfix-expression such as a->f(...)and a.b(...) are the part of them. See https://timsong-cpp.github.io/cppwp/n4659/expr.call#5

In the Boost.Asio, the following style asynchronous member function call is typical patter.

auto sp_object = std::make_shared<object>(...);
sp_object->async_func(
    params,
    [sp_object]
    (boost::syste_error_code const&e, ...) {
        if (e) return;
        sp_object->other_async_func(
            params,
            [sp_object]
            (boost::syste_error_code const&e, ...) {
                if (e) return;
                // do some
            }
        );
    }
);

I'd like to clarify the following three cases's safety.

Case1: shared_ptr move and member function

auto sp_object = std::make_shared<object>(...);
sp_object->async_func(
    params,
    [sp_object = std::move(sp_object)]
    (boost::syste_error_code const&e, ...)  mutable { // mutable is for move
        if (e) return;
        sp_object->other_async_func(
            params,
            [sp_object = std::move(sp_object)]
            (boost::syste_error_code const&e, ...) {
                if (e) return;
                // do some
            }
        );
    }
);

This pattern is like https://www.boost.org/doc/libs/1_70_0/doc/html/boost_asio/reference/basic_stream_socket/async_read_some.html

I think it is safe because the postfix-expression -> is evaluated before sp_object = std::move(sp_object).

Case2: value move and member function

some_type object(...);
object.async_func(
    params,
    [object = std::move(object)]
    (boost::syste_error_code const&e, ...)  mutable { // mutable is for move
        if (e) return;
        object.other_async_func(
            params,
            [object = std::move(object)]
            (boost::syste_error_code const&e, ...) {
                if (e) return;
                // do some
            }
        );
    }
);

I think is is dangerous because even if the postfix-expression . is evaluated before object = std::move(object), async_func may access the member of object.

Case3: shared_ptr move and free function

auto sp_object = std::make_shared<object>(...);
async_func(
    *sp_object,
    params,
    [sp_object = std::move(sp_object)]
    (boost::syste_error_code const&e, ...)  mutable { // mutable is for move
        if (e) return;
        other_async_func(
            *sp_object,
            params,
            [sp_object = std::move(sp_object)]
            (boost::syste_error_code const&e, ...) {
                if (e) return;
                // do some
            }
        );
    }
);

This pattern is like https://www.boost.org/doc/libs/1_70_0/doc/html/boost_asio/reference/async_read/overload1.html

I think it is dangerous because there is no postfix-expression. So sp_object could be moved by third argument move capture before dereference as *sp_object by the first argument.

conclusion

Only case1 is safe and others are dangerous (undefined behavior). I need to be careful that It is unsafe on C++14 and older compilers. It can speed up calling asynchronous member function because shared_ptr's atomic counter operation is not happened. See Why would I std::move an std::shared_ptr? But I also need to consider that advantage could be ignored, it is depends on the application.

Am I understanding correctly about C++17 evaluation order change (precise definition) and asynchronous operation relationship?

Answer 1

Answer

Thanks to Explorer_N 's comments. I got the answer.

I asked that "Case1 is safe but Case2 and Case3 are unsafe is that rgiht?". However, Case1 is safe if and only if a constraint I wrote later (*1) is satisfied. That means Case1 is unsafe in general.

It is depends on async_func()

Here is an unsafe case:

#include <iostream>
#include <memory>
#include <boost/asio.hpp>

struct object : std::enable_shared_from_this<object> {
    object(boost::asio::io_context& ioc):ioc(ioc) {
        std::cout << "object constructor this: " << this << std::endl;
    }

    template <typename Handler>
    void async_func(Handler&& h) {
        std::cout << "this in async_func: " << this << std::endl;
        h(123); // how about here?
        std::cout << "call shared_from_this in async_func: " << this << std::endl;
        auto sp = shared_from_this();
        std::cout << "sp->get() in async_func: " << sp.get() << std::endl;
    }

    template <typename Handler>
    void other_async_func(Handler&& h) {
        std::cout << "this in other_async_func: " << this << std::endl;
        h(123); // how about here?
        std::cout << "call shared_from_this in other_async_func: " << this << std::endl;
        auto sp = shared_from_this();
        std::cout << "sp->get() in other_async_func: " << sp.get() << std::endl;
    }

    boost::asio::io_context& ioc;
};

int main() {
    boost::asio::io_context ioc;
    auto sp_object = std::make_shared<object>(ioc);

    sp_object->async_func(
        [sp_object = std::move(sp_object)]
        (int v) mutable { // mutable is for move
            std::cout << v << std::endl;
            sp_object->other_async_func(
                [sp_object = std::move(sp_object)]
                (int v) {
                    std::cout << v << std::endl;
                }
            );
        }
    );
    ioc.run();
}

Running demo https://wandbox.org/permlink/uk74ACox5EEvt14o

I considered why the first shared_from_this() is ok but second call throws std::bad_weak_ptr in the code above. That is because the callback handler is called from the async_func and other_async_func directly. The move happens twice. So that the first level (async_func) shared_from_this is failed.

Even if the callback handler is NOT called from async function directly, it is still unsafe on multi-threaded case.

Here is an unsafe code:

#include <iostream>
#include <memory>
#include <boost/asio.hpp>

struct object : std::enable_shared_from_this<object> {
    object(boost::asio::io_context& ioc):ioc(ioc) {
        std::cout << "object constructor this: " << this << std::endl;
    }

    template <typename Handler>
    void async_func(Handler&& h) {
        std::cout << "this in async_func: " << this << std::endl;

        ioc.post(
            [this, h = std::forward<Handler>(h)] () mutable {
                h(123);
                sleep(1);
                auto sp = shared_from_this();
                std::cout << "sp->get() in async_func: " << sp.get() << std::endl;
            }
        );
    }

    template <typename Handler>
    void other_async_func(Handler&& h) {
        std::cout << "this in other_async_func: " << this << std::endl;

        ioc.post(
            [this, h = std::forward<Handler>(h)] () {
                h(456);
                auto sp = shared_from_this();
                std::cout << "sp->get() in other_async_func: " << sp.get() << std::endl;
            }
        );
    }

    boost::asio::io_context& ioc;
};

int main() {
    boost::asio::io_context ioc;
    auto sp_object = std::make_shared<object>(ioc);

    sp_object->async_func(
        [sp_object = std::move(sp_object)]
        (int v) mutable { // mutable is for move
            std::cout << v << std::endl;
            sp_object->other_async_func(
                [sp_object = std::move(sp_object)]
                (int v) {
                    std::cout << v << std::endl;
                }
            );
        }
    );
    std::vector<std::thread> ths;
    ths.reserve(2);
    for (std::size_t i = 0; i != 2; ++i) {
        ths.emplace_back(
            [&ioc] {
                ioc.run();
            }
        );
    }
    for (auto& t : ths) t.join();
}

Running Demo: https://wandbox.org/permlink/xjLZWoLdn8xL89QJ

Constraint of case1 is safe

*1 However, in the case1, if and only if struct object doesn't expect it is held by shared_ptr, it is safe. In other words, as long as struct object doesn't use shared_from_this mechanism, it is safe.

Another way to control the sequence. (C++14 supported)

If and only if the constraint above is satisfied, we can control the evaluation sequence without C++17 sequence definition. It supports both case1 and case3. Simply get reference of the pointee object that is held by shared_ptr. The key point is pointee object is preserved even if the shared_ptr is moved. So get the reference of pointee object before the shared_ptr moved, and then shared_ptr is moved, the pointee object is not affected.

However, shared_from_this is exceptional case. It uses shared_ptr mechanism directly. So that is affected by shared_ptr moving. Hence it is unsafe. That is the reason of the constraint.

Case1

// The class of sp_object class doesn't use shared_from_this mechanism
auto sp_object = std::make_shared<object>(...);
auto& r = *sp_object;
r.async_func(
    params,
    [sp_object]
    (boost::syste_error_code const&e, ...) {
        if (e) return;
        auto& r = *sp_object;
        r.other_async_func(
            params,
            [sp_object]
            (boost::syste_error_code const&e, ...) {
                if (e) return;
                // do some
            }
        );
    }
);

Case3

// The class of sp_object class doesn't use shared_from_this mechanism
auto sp_object = std::make_shared<object>(...);
auto& r = *sp_object;
async_func(
    r,
    params,
    [sp_object = std::move(sp_object)]
    (boost::syste_error_code const&e, ...)  mutable { // mutable is for move
        if (e) return;
        auto& r = *sp_object;
        other_async_func(
            r,
            params,
            [sp_object = std::move(sp_object)]
            (boost::syste_error_code const&e, ...) {
                if (e) return;
                // do some
            }
        );
    }
);

Answer 2

Your question can be greatly simplified to "is the following safe":

some_object.foo([bound_object = std::move(some_object)]() {
    bound_object.bar()
});

From your linked question, the standard says

All side effects of argument evaluations are sequenced before the function is entered

One such side-effect is the move from some_object - so this is equivalent to:

auto callback = [bound_object = std::move(some_object)]() {
    bound_object.bar()
}
some_object.foo(std::move(callback));

Clearly, this moves out of some_object before the foo method is called. This is safe if and only if foo is called on a moved-from object.

---

Using this knowledge:

• Case 1 will likely segfault and is definitely not safe, because calling operator->() on a moved-from shared_ptr returns nullptr, which you then call ->async_func on.
• Case 2 is safe only if calling async_func on a moved-from some_type is safe, but it's very unlikely it does what you intend unless the type doesn't actually define a move constructor.
• Case 3 is not safe because while it's ok to move a shared pointer after dereferencing it (as moving a shared pointer does not change the object it points to), C++ makes no guarantee about which function argument will be evaluated first.