3

When I use Boost.Asio, creating object such as ip::tcp::socket or deadline_timer as std::shared_ptr and copy captured it to the completion handler as lambda expression.

I curious that what happens if I use move capture instead of copy capture. I think that it is dangerous. In the following example, I think that tim = std::move(tim) is evaluated before tim->async_wait. So tim no longer has valid pointer. It is my guess. In order to trace std::shared_ptr's behavior, I created std::shared_ptr wrapper shared_ptr.

#include <iostream>

#include <boost/asio.hpp>

namespace as = boost::asio;

template <typename... Args>
struct shared_ptr : std::shared_ptr<Args...> {
    using base = std::shared_ptr<Args...>;
    using base::base; // inheriting constructor

    shared_ptr(shared_ptr&& other) : base(std::move(other)) {
        std::cout << "move" << std::endl;
    }
    typename base::element_type* operator->() {
        std::cout << "->" << std::endl;
        return base::get();
    }
};

int main() {
    as::io_context ioc;

    ioc.post( 
        [&] {
            shared_ptr<as::deadline_timer> tim(new as::deadline_timer(ioc));
            tim->expires_from_now(boost::posix_time::seconds(1));
            tim->async_wait( 
                // I think that it is dangerous because tim has been moved before tim->async_wait()
                [&, tim = std::move(tim)] 
                    std::cout << ec.message() << std::endl;
                }
            );
        }
    );

    ioc.run();
}

I run the code on several environment:

All option is -std=c++14

g++ 7.1.0 or later : https://wandbox.org/permlink/rgJLp66vH7jKodQ8 A

g++ 6.3.0 : https://wandbox.org/permlink/XTIBhjJSqmkQHN4P B

clang++ 4.0.1~ : https://wandbox.org/permlink/nEZpFV874pKstjHA A

Output A

->
->
move
move
move
move
Success

Output B

->
move
->
Segmentation fault

It seems that clang++ and g++ 7.1.0 or later evaluates tim->async_wait() first.

g++ 6.3.0 evaluates tim = std::move(tim) first.

Is this an undefined behavior? Or evaluation order is defined at some point?

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
Takatoshi Kondo
  • 3,111
  • 17
  • 36
  • 2
    I currently don't have the standards wording, but I'm pretty sure this is undefined behaviour because you're `tim->async_wait` call acts on an object which has been moved from. – jan.sende Jul 19 '19 at 08:24

1 Answers1

2

The evaluation order in C++17 is well-defined, such that the expression leading to the function call (tim->async_wait) is sequenced before any of its arguments.

C++14 however, this is unspecified, due to lack of such sequencing rules. That is, it may work, it may not, and implementations aren't required to tell you which way it picks, or even to be consistent from one call to another.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • Thank you for the answer. That means if the compiler fully support C++17, `tim = std::move(tim)` is better choice. Because of shared count is not changed, it is higher performance (speed) than copy implementation. Is that right? – Takatoshi Kondo Jul 19 '19 at 14:11
  • 1
    @TakatoshiKondo: Performance is kind of irrelevant in this context, since you're about to perform an asynchronous dispatch operation anyway. I doubt the atomic increment/decrements are going to be the difference maker in terms of performance. – Nicol Bolas Jul 19 '19 at 14:13
  • You mean you think that atomic increment/decrement makes performance difference in general, but it is not a dominant factor in this case. Am I understanding correctly? – Takatoshi Kondo Jul 19 '19 at 15:23
  • 1
    It doesn't matter that `tim->async_wait` is sequenced before its arguments if it's a member function - it will still be called with the pre-move `this` pointer, after the `move` has already occurred. – Eric Jul 23 '19 at 04:53
  • @Eric: It's not about how it works if it is sequenced before. Indeed, it *needs* to be sequenced before to work. The problem is if it is sequenced *after*. After the arguments are evaluated, `tim` no longer has a valid pointer. So you have to fetch the pointer *before* evaluating the arguments, or the thing doesn't work. – Nicol Bolas Jul 23 '19 at 13:19