-2

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?

Improve this question
11
  • firstly, why do you think Case:1 is safe? though sp_object->async_func evaluated before sp_object = std::move(sp_object)....whether it is undefined or not is totally depends on async_func's definition, what do you think would happen when async_func access shared_from_this()? Commented Jul 22, 2019 at 8:10
  • Because sp_object = std::move(sp_object) keep the same pointee object. Moved to sp_object.get() returns the same address (object) of this is async_func(). So I believe that shared_from_this() works fine. Here is working example wandbox.org/permlink/NooUkn4SUSAOPLDU Commented Jul 22, 2019 at 8:54
  • in-case of moving shared_ptr this is what std do have to say "10) Move-constructs a shared_ptr from r. After the construction, *this contains a copy of the previous state of r, r is empty and its stored pointer is null. The template overload doesn't participate in overload resolution if Y* is not implicitly convertible to (until C++17)compatible with (since C++17) T*."....so not sure what you've been seeing is well-defined. Commented Jul 22, 2019 at 9:13
  • I think that timsong-cpp.github.io/cppwp/n4659/expr.call#5 means at sp_object-> is replaced to the address of sp_object.get(). Let's say the address is addr_object. So that means it is replaced as addr_object->async_read. After this replacing, sp_object become empty. But it is no problem. Commented Jul 22, 2019 at 9:37
  • Have changed your example a little bit ( as to me, it behaviour depends on what the function we invoke does ) wandbox.org/permlink/Tv9pbhvls2ZkHJE7 , let me know if I missed anything. got this "terminating with uncaught exception of type std::__1::bad_weak_ptr: bad_weak_ptr" Commented Jul 22, 2019 at 9:54

2 Answers 2

Reset to default
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
            }
        );
    }
);
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

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.
Improve this answer

7 Comments

Do you mean the first code and the second code are equivalent ?
Updated to explicitly cover your cases
@TakatoshiKondo you can accept this as an answer, it clearly states evaluation comes before execution and that to me sounds a key thing.
In your 1st example, foo() is called with moved from object. This is running code: wandbox.org/permlink/tDxm3LMiVyzAeT9J If foo() is not safe called with moved from object, it is unsafe. I agree this point. It is case2 of my question. In case1, some_object is shared_ptr. See wandbox.org/permlink/domM8hS1JvuxqUs0 If and only if some_class doesn't expect "I am held by shared_ptr", I think this is safe. I think that case1 with above constraint is safe. That is my point.
@TakatoshiKondo: You're right, case 1 looks safe after all based on that reasoning
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.