boost::asio::ip::tcp::acceptor terminates application when receiving connection request using async_accept

Question

I want to make this simple server that listens to incoming connection requests, makes connections and sends some data. When I start this acceptor looks like it's working fine, it waits for those incoming connection requests, but when my client tries to connect to this acceptor it automatically crushes. I cant even catch any exceptions with catch(...)

When I start this program it looks like this in a terminal

But when I try to connect

Client application received this kind of error code

Is there something fundamentally wrong with my my_acceptor class?

class my_acceptor{
public:
    my_acceptor(asio::io_context& ios, unsigned short port_num) :
        m_ios(ios),
        port{port_num},
        m_acceptor{ios}{}

    //start accepting incoming connection requests
    void Start()
    {
        std::cout << "Acceptor Start" << std::endl;
        boost::asio::ip::tcp::endpoint endpoint(boost::asio::ip::tcp::v4(), port);
        m_acceptor.open(endpoint.protocol());
        m_acceptor.set_option(boost::asio::ip::tcp::acceptor::reuse_address(true));
        m_acceptor.bind(endpoint);
        m_acceptor.listen();

        InitAccept();
    }

    void Stop(){}

private:
    void InitAccept()
    {
        std::cout << "Acceptor InitAccept" << std::endl;

        std::shared_ptr<asio::ip::tcp::socket> sock{new asio::ip::tcp::socket(m_ios)};

        m_acceptor.async_accept(*sock.get(),
            [this, sock](const boost::system::error_code& error)
            {
                onAccept(error, sock);
            });
    }

    void onAccept(const boost::system::error_code& ec, std::shared_ptr<asio::ip::tcp::socket> sock)
    {
        std::cout << "Acceptor onAccept" << std::endl;
    }

private:
    unsigned short port;
    asio::io_context& m_ios;
    asio::ip::tcp::acceptor m_acceptor;

};

Just in case this is the Server code that wraps my_acceptor

class Server{
public:
    Server(){}

    //start the server
    void Start(unsigned short port_num, unsigned int thread_pool_size)
    {
        assert(thread_pool_size > 0);

        //create specified number of threads and add them to the pool
        for(unsigned int i = 0; i < thread_pool_size; ++i)
        {
            std::unique_ptr<std::thread> th(
                new std::thread([this]()
                {
                    m_ios.run();
                }));

            m_thread_pool.push_back(std::move(th));
        }

        //create and start acceptor
        acc.reset(new my_acceptor(m_ios, port_num));
        acc->Start();
    }

    //stop the server
    void Stop()
    {
        work_guard.reset();
        acc->Stop();
        m_ios.stop();


        for(auto& th : m_thread_pool)
        {
            th->join();
        }
    }

private:
    asio::io_context m_ios;
    boost::asio::executor_work_guard<boost::asio::io_context::executor_type> work_guard = boost::asio::make_work_guard(m_ios);
    std::unique_ptr<my_acceptor> acc;
    std::vector<std::unique_ptr<std::thread>> m_thread_pool;
};

Answer 1

There's a threading bug, at least. tcp::acceptor is not thread-safe and you (potentially) run multiple threads. So you will need to make the acceptor access be done from a strand.

my_acceptor(asio::io_context& ios, unsigned short port_num) :
    m_ios(ios),
    port{port_num},
    m_acceptor{make_strand(ios)}{}

And then any operation involving it must be on that strand. E.g., the missing Stop() code should look like:

void Stop(){
    post(m_acceptor.get_executor(), [this] { m_acceptor.cancel(); });
}

I leave the initial accept as-is because at that point there aren't multiple threads involved.

Likewise in Start() and Stop() you should check whether acc is null, because acc->Stop() would throw and just replacing a running acc would cause Undefined Behaviour due to deleting the instance that is still having async operations in flight.

In a sidenote, m_ios.stop() should not be necessary if you stop the running acceptor. In the future you might have to signal any client connections to stop, in order for the threads to naturally join.

Here's how I'd complete the accept loop:

void onAccept(error_code ec, std::shared_ptr<tcp::socket> sock)
{
    std::cout << "Acceptor onAccept " << ec.message() << " " << sock.get() << std::endl;
    if (!ec) {
        InitAccept();
    }
}

Note how unless the socket is canceled (or otherwise in error), we keep accepting.

I think the threading issue was likely your big problem. The result after my suggestions works:

Live On Coliru

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

#include <thread>

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

class my_acceptor {
public:
    my_acceptor(asio::io_context& ios, unsigned short port_num) :
        m_ios(ios),
        port{port_num},
        m_acceptor{make_strand(ios)}{}

    //start accepting incoming connection requests
    void Start()
    {
        std::cout << "Acceptor Start" << std::endl;
        tcp::endpoint endpoint(tcp::v4(), port);
        m_acceptor.open(endpoint.protocol());
        m_acceptor.set_option(tcp::acceptor::reuse_address(true));
        m_acceptor.bind(endpoint);
        m_acceptor.listen();

        InitAccept();
    }

    void Stop(){
        post(m_acceptor.get_executor(), [this] { m_acceptor.cancel(); });
    }

private:
    void InitAccept()
    {
        std::cout << "Acceptor InitAccept" << std::endl;

        auto sock = std::make_shared<tcp::socket>(m_ios);

        m_acceptor.async_accept(*sock,
            [this, sock](error_code error) { onAccept(error, sock); });
    }

    void onAccept(error_code ec, const std::shared_ptr<tcp::socket>& sock)
    {
        std::cout << "Acceptor onAccept " << ec.message() << " " << sock.get() << std::endl;
        if (!ec) {
            InitAccept();
        }
    }

private:
    asio::io_context& m_ios;
    unsigned short port;
    tcp::acceptor m_acceptor;
};

class Server{
public:
    Server() = default;

    //start the server
    void Start(unsigned short port_num, unsigned int thread_pool_size)
    {
        assert(!acc); // otherwise UB results
        assert(thread_pool_size > 0);

        //create specified number of threads and add them to the pool
        for(unsigned int i = 0; i < thread_pool_size; ++i)
        {
            std::unique_ptr<std::thread> th(
                new std::thread([this]() { m_ios.run(); }));

            m_thread_pool.push_back(std::move(th));
        }
        //create and start acceptor
        acc = std::make_unique<my_acceptor>(m_ios, port_num);
        acc->Start();
    }

    //stop the server
    void Stop()
    {
        work_guard.reset();
        if (acc) {
            acc->Stop();
        }
        //m_ios.stop();

        for(auto& th : m_thread_pool) {
            th->join();
        }
        acc.reset();
    }

private:
    asio::io_context m_ios;
    asio::executor_work_guard<asio::io_context::executor_type>
        work_guard = make_work_guard(m_ios);
    std::unique_ptr<my_acceptor> acc;
    std::vector<std::unique_ptr<std::thread>> m_thread_pool;
};

int main() {
    Server s;
    s.Start(6868, 1);
    std::this_thread::sleep_for(10s);
    s.Stop();
}

Testing with netcat as client:

for msg in one two three; do
    sleep 1
    nc 127.0.0.1 6868 <<< "$msg"
done

Prints

Acceptor Start
Acceptor InitAccept
Acceptor onAccept Success 0x1f26960
Acceptor InitAccept
Acceptor onAccept Success 0x7f59f80009d0
Acceptor InitAccept
Acceptor onAccept Success 0x7f59f8000a50
Acceptor InitAccept
Acceptor onAccept Operation canceled 0x7f59f80009d0