0

I'm trying to pass a socket along a connection handshake, and use std::bind to do so. The compile issue I'm getting (in one continuous block, which I've added spaces to for readability) is:

'std::_Bind<_Functor(_Bound_args ...)>::_Bind(_Functor&&, _Args&& ...) 

[with _Args = {socket_state**, std::function<void(socket_state*)>&, boost::asio::basic_socket_acceptor<boost::asio::ip::tcp, boost::asio::executor>&, boost::asio::io_context&}; 

_Functor = void (*)(socket_state*, std::function<void(socket_state*)>&, boost::asio::basic_socket_acceptor<boost::asio::ip::tcp>&, boost::asio::io_context&); 

_Bound_args = {socket_state**, std::function<void(socket_state*)>, boost::asio::basic_socket_acceptor<boost::asio::ip::tcp, boost::asio::executor>, boost::asio::io_context}]':

My code is below, with the error appearing to nag at the std::bind arguments given to boost::asio::acceptor.async_accept(socket, ...) and the parameters for the accept_new_client method

    void start_server(std::function<void(socket_state*)>& func, tcp::acceptor& acceptor, boost::asio::io_context& context)
    {
        acceptor.listen();
        // Start client connection loop
        networking::wait_for_client(func, acceptor, context);
    }

    void wait_for_client(std::function<void(socket_state*)>& func, tcp::acceptor& acceptor, boost::asio::io_context& context)
    {
        boost::asio::ip::tcp::socket socket(context);

        // socket_state is its own class which links a particular socket with an ID and buffer data
        // it also holds a function to indicate which part of the connection handshake it needs to go to next
        socket_state* state = new socket_state(func, &socket);
        acceptor.async_accept(socket, std::bind(&networking::accept_new_client, state, func, acceptor, context));
    }

    void accept_new_client(socket_state* state, std::function<void(socket_state*)>& func, tcp::acceptor& acceptor, boost::asio::io_context& context)
    {
            state->on_network_action(state);
            wait_for_client(func, acceptor, context);
    }

It seems like they would match, but you can see the error state my std::bind arguments are socket_state** instead of socket_state*, and boost::asio::basic_socket_acceptor<boost::asio::ip::tcp, boost::asio::executor>& instead of boost::asio::basic_socket_acceptor<boost::asio::ip::tcp>&.

I have no idea what the "with _Args" vs. "_Bound_args" is either.

12
  • It's very suspicious that you are storing a pointer to the local variable socket in a structure allocated on the heap. As soon as wait_for_client returns, that local variable will be destroyed, leaving socket_state with a dangling pointer. Commented May 1, 2021 at 4:20
  • 1
    The error message suggests that you are passing socket_state** (two stars) to std::bind, while the function you are binding expects socket_state* (one star). The error message doesn't match the code shown; it must be different from the code you are actually compiling. Commented May 1, 2021 at 4:24
  • I'm very confused because the exact code snippet is what is being compiled. I've made changes to the snippet (like adding & behind some of the std::bind arguments) and it changes the error message, so it must be direct to the code I'm compiling. Also would storing a smart pointer instead be the move? Commented May 1, 2021 at 5:34
  • 1
    What's the exact and complete error message? You only showed a part of it - missing is the actual message where the compiler says what it believes is wrong. I'm not familiar with boost::asio, but a quick glance appears to suggest that the callback is expected to take const boost::system::error_code& error parameter; yours doesn't. Perhaps this is the source of the error. Commented May 1, 2021 at 11:48
  • 1
    @sehe networking is a namespace which serves as an abstraction for a server.cpp class to make use of. @IgorTandetnik The full error was literal hundreds of lines; the snippet I posted was the first portion and seemed likely to be the root of where everything else trickled down. In any case it is resolved now with the help of sehe. I appreciate your discussion here, it was insightful! Commented May 1, 2021 at 14:24

1 Answer 1

1

There's many problems in this code.

The shared pointer seems to be at the wrong level of abstraction. You would want the entire "connection" type to be of shared lifetime, not just the socket. In your case, socket_state is a good candidate.

Regardless, your socket is a local variable that you pass a stale pointer to inside socket_state. Socket-state looks like it will necessarily be leaked.

So that will never work already.

Next up, the bind is binding all parameters eagerly, leaving a nullary signature. That's not what any overload accepts [no pun intended]. You need to match

Let's go for AcceptHandler. Also, let's not bind all the redundant args (func was already in the socket_stateremember,io_context` is overshared etc.).

In general it looks like you need to develop confidence in knowing where your state is. E.g. this line is is symptomatic:

state->on_network_action(state);

Since on_network_action is a member function of socket_state, there should never be any need to pass the state as an argument (it will be this implicitly). The same thing goes for acceptor and contest in all occurrences.

Demo

Fixing all the above, using std::shared_ptr and bind (you already did), notice the placeholder::_1 to accept the error_code etc.)

Live On Coliru

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

namespace ba = boost::asio;
using namespace std::chrono_literals;
using boost::system::error_code;
using ba::ip::tcp;

struct socket_state;
using Callback = std::function<void(socket_state&)>;

struct socket_state : std::enable_shared_from_this<socket_state> {
    Callback _callback;
    tcp::socket _socket;

    template <typename Executor>
    socket_state(Callback cb, Executor ex) : _callback(cb)
                                           , _socket(ex)
    {
    }

    void on_network_action() {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }
};

struct networking {
    using StatePtr = std::shared_ptr<socket_state>;

    explicit networking(ba::io_context& ctx, Callback callback)
        : context(ctx)
        , callback(callback)
    {
    }

    ba::io_context& context;
    tcp::acceptor acceptor {context, {{}, 8989}};
    Callback callback;

    void start_server()
    {
        std::cout << "start_server" << std::endl;
        acceptor.listen();
        wait_for_client(); // Start client connection loop
    }

    void stop_server() {
        std::cout << "stop_server" << std::endl;
        acceptor.cancel();
        acceptor.close();
    }

    void wait_for_client()
    {
        std::cout << "wait_for_client" << std::endl;
        // socket_state is its own class which links a particular socket with
        // an ID and buffer data it also holds a function to indicate which
        // part of the connection handshake it needs to go to next
        auto state =
            std::make_shared<socket_state>(callback, context.get_executor());

        acceptor.async_accept(state->_socket,
                              std::bind(&networking::accept_new_client, this,
                                        std::placeholders::_1, state));
    }

    void accept_new_client(error_code ec, StatePtr state)
    {
        if (ec) {
            std::cout << "accept_new_client " << ec.message() << std::endl;
            return;
        }
        std::cout << "accept_new_client " << state->_socket.remote_endpoint()
                  << std::endl;
        state->on_network_action();
        wait_for_client();
    }
};

int main() {
    ba::io_context ctx;
    networking server(ctx, [](socket_state&) {
        std::cout << "This is our callback" << std::endl;
    });

    server.start_server();

    ctx.run_for(5s);

    server.stop_server();
    ctx.run();
}

With some random connections:

start_server
wait_for_client
accept_new_client 127.0.0.1:54376
void socket_state::on_network_action()
wait_for_client
accept_new_client 127.0.0.1:54378
void socket_state::on_network_action()
wait_for_client
accept_new_client 127.0.0.1:54380
void socket_state::on_network_action()
wait_for_client
accept_new_client 127.0.0.1:54382
void socket_state::on_network_action()
wait_for_client
stop_server
accept_new_client Operation canceled

Note that version makes the comments

// socket_state is its own class which links a particular socket with
// an ID and buffer data it also holds a function to indicate which
// part of the connection handshake it needs to go to next

no longer complete lies :)

Sign up to request clarification or add additional context in comments.

2 Comments

If you've made it this far, maybe you'll enjoy a version with some judicious renames, not hardcoding the port in the Server, more consistent executor use, illustrating how to initiate a strand per session: coliru.stacked-crooked.com/a/a2517741f7d28074
"In general it looks like you need to develop confidence in knowing where your state is" - indeed, before I was hazily writing what seemed good with no real footing of what I was doing. It feels like this answer has put my footing on the ground in terms of actually seeing what is going on with my socket_state and others (io_context and acceptors). Thank you for the clear and thorough answer.

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.