Using lambdas with auto declaration vs in-place?

Question

I'm trying to learn modern C++ and I'm using Boost.Asio for networking. I wrote a TCP connection class, which uses Asio's asynchronous operations. This is currently my method for reading data from a socket:

template<class T>
inline auto connection<T>::read(size_t length) -> void
{
    auto handler = [&](const boost::system::error_code& error, size_t bytes_transferred) {
        if (error == boost::asio::error::eof or error == boost::asio::error::connection_reset) {
            close();
        } else {
            on_read(bytes_transferred);
        }
    };
    socket.async_read_some(boost::asio::buffer(read_buffer, length), handler);
}

Here I declared the read handler separately with auto, because I think it looks more readable than an in-place lambda, i.e.

template<class T>
inline auto connection<T>::read(size_t length) -> void
{
    socket.async_read_some(boost::asio::buffer(read_buffer, length), [&](const boost::system::error_code& error, size_t bytes_transferred) {
        if (error == boost::asio::error::eof or error == boost::asio::error::connection_reset) {
            close();
        } else {
            on_read(bytes_transferred);
        }
    });
}

However I ran into a segmentation fault with the first version, and I believe this is because the handler lambda is lost when the method goes out of scope. Then I tried to move the handler with std::move

socket.async_read_some(boost::asio::buffer(read_buffer, length), std::move(handler));

which seems to fix the segfault.

Now my question is: Are there any performance or other issues with using the first version (with std::move) vs in-place? Which one do you think is better practice?

Using a lambda inline is the typical use-case in this kind of situation. As far as readability goes, I prefer to start an inline lambda on a new (indented) line, eg `socket.async_read_some(...,[&](...) { ... });` to show the lambda is a part of, but inside, the outer call. Using a variable to hold a lambda is useful only when you need to refer to or call the lambda multiple times. — Remy Lebeau, Feb 21 '19 at 00:54
Thank you for the input. Using a newline would indeed be better and I thought about it, but I've also placed some coding style rules to myself where I use 8 spaces for indentation with max 3 indentation levels. In this case I'd either have to use 4 indentation levels or start the lambda on the same line as the ```socket.async..``` and neither of them really look good. I guess I could also declare some internal handle_on_read method using boost::bind instead of a lambda (on_read is virtual and overwritten by deriving classes for specific use-cases). — Antti L., Feb 21 '19 at 01:38
I don't think limiting yourself to 3 max indentation levels is very realistic. Non-trivial functions in production code can easily exceed that limit. If you need an indent, especially for readability, just use an ident, don't limit yourself. The compiler certainly won't limit you. And 8 spaces for an indent is pretty large, 4 is more commonly used, or use tabs instead of spaces and then adjust the tab width as desired. — Remy Lebeau, Feb 21 '19 at 01:51
That is probably true and I'll think about it. The 8 spaces and 3 indentation levels rules actually come from the Linux kernel coding style. I know they're for C, but I agree with them and I think they increase readability. At first I started with CamelCase and 2 spaces and quickly realized how horrible it was to read. Thanks again. — Antti L., Feb 21 '19 at 02:07
Is this really how `async_read_some` is supposed to behave? From this description it appears that for lvalue argument this method stores a reference to the actual handler, while for the xvalue argument it moves the handler. — AnT stands with Russia, Feb 21 '19 at 02:20
Asynchronous initiating functions in Networking TS (and by extension, Asio and Boost.Asio) takes ownership of handlers by performing a "decay-copy." That means the handler is either moved from, or copied, depending on whether the argument is an xvalue. — Vinnie Falco, Feb 21 '19 at 02:24
@Vinnie Falco: Why is the first version crashing then? It is supposed to copy the handler, thus becoming independent from the lifetime of local `handler` object. — AnT stands with Russia, Feb 21 '19 at 02:25

score 1 · Accepted Answer · answered Feb 21 '19 at 02:32

Both of these code examples should work. The first example passes the handler as an lvalue, in which case the implementation will make a copy. The second example passes a lambda as a prvalue, in which case the implementation will perform a move-construction. As both the lvalue and prvalue are trivial, the two operations are the same.

Asynchronous initiating functions in Networking TS (and by extension, Asio and Boost.Asio) take ownership of handlers by performing a "decay-copy." That means the handler is either copied or moved from depending on whether the argument is an lvalue or not.

I am not sure why your first example crashes, but it has nothing to do with the lifetime of the lambda. For obvious reasons, asynchronous initiating functions never receive the handle by reference, and always take ownership by decay-copy.

There must be some other problem with your code, in the part that you haven't pasted. For example, what is keeping the connection object alive after the function returns?

Okay I tried running my program a couple more times and you're right, there's still a segfault. It was just pure luck that it didn't happen again earlier. [I used gdb and printed the backtrace](https://gitlab.com/weftworks/common/snippets/1827673). I'm also using boost::log and the backtrace showed boost::log calls, but I didn't suspect them at all and left my logging macros out from my first post. I don't have experience with gdb, so is #0 where the segfault occurs? Now I compiled my program with logging completely disabled and this time it really looks like the segfault is fixed. — Antti L., Feb 21 '19 at 03:12
So now I need to take a look at my logger wrapper class and macros to figure out what's wrong with them. Edit: Here's my logger [header](https://gitlab.com/weftworks/common/blob/master/include/weftworks/logger.hpp) and [implementation](https://gitlab.com/weftworks/common/blob/master/src/logger/logger.ipp). I've read that macros are evil, but they're so convenient for logging. Going to look for alternative solutions. Thanks for the help! — Antti L., Feb 21 '19 at 03:16

Using lambdas with auto declaration vs in-place?

1 Answers1