Chnossos
Chnossos

Reputation: 10486

Boost.Asio TCP moved-to socket destructor not enough to cleanly close?

Consider this test program :

#include <boost/asio/io_service.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <functional>
#include <iostream>

static void callback (boost::asio::ip::tcp::socket && socket)
{
    //boost::asio::ip::tcp::socket new_socket = std::move(socket);
    std::cout << "Accepted" << std::endl;
}

static void on_accept (boost::asio::ip::tcp::acceptor &  acceptor,
                       boost::asio::ip::tcp::socket &    socket,
                       boost::system::error_code const & error)
{
    if (error)
    {
        std::cerr << error << ' ' << error.message() << std::endl;
        return ;
    }

    callback(std::move(socket));
    acceptor.async_accept
    (
        socket,
        std::bind(on_accept, std::ref(acceptor), std::ref(socket), std::placeholders::_1)
    );
}

int main ()
{
    boost::asio::io_service        service;
    boost::asio::io_service::work  work { service };
    boost::asio::ip::tcp::acceptor acceptor { service };
    boost::asio::ip::tcp::socket   socket { service };

    boost::asio::ip::tcp::endpoint endpoint { boost::asio::ip::tcp::v4(), 5555 };
    boost::system::error_code      ec;

    using socket_base = boost::asio::socket_base;
    auto option = socket_base::reuse_address { false };

    if (acceptor.open(endpoint.protocol(), ec) ||
        acceptor.set_option(option, ec) ||
        acceptor.bind(endpoint, ec) ||
        acceptor.listen(socket_base::max_connections, ec) ||
        acceptor.is_open() == false)
        return 1;

    acceptor.async_accept
    (
        socket,
        std::bind(on_accept, std::ref(acceptor), std::ref(socket), std::placeholders::_1)
    );

    service.run();
}

When I connect a client to it, I get an error :

Accepted
system:1 Incorrect function

(The on_accept() function is called with an error code when the socket object from the callback() function is destroyed).

Also, the client is not disconnected at all.

If I uncomment the line in the callback() function, everything works fine, no error message and the client is disconnected as expected.

Now for the environment settings, I'm under Windows 8.1, using a MinGW-w64 v4.9.2 compiler with Boost.Asio v1.58.0 compiled with that same compiler.

The command line used to compile the file is as follow :

$ g++ -std=c++14 -IC:/C++/boost/1.58.0 main.cpp -LC:/C++/boost/1.58.0/lib -lboost_system-mgw49-mt-1_58 -lwsock32 -lws2_32 -o test.exe

Note that using Boost 1.57.0 results in the same behavior.

I can also remove the commented line completely, and then use this :

static void callback (boost::asio::ip::tcp::socket && socket)
{
    std::cout << "Accepted" << std::endl;
    socket.shutdown(socket.shutdown_both);
    socket.close();
}

And the program will behave correctly too.


So, how come I need to add extra steps to not get an error here ? IIRC this behavior wasn't there a couple of months ago when I first tried that program.

Upvotes: 1

Views: 1322

Answers (1)

Tanner Sansbury
Tanner Sansbury

Reputation: 51881

The code only creates a single socket, which is an automatic variable whose lifetime will end once main() returns. std::move(socket) only returns an xvalue that can be provided to socket's move constructor; it does not construct a socket.

To resolve this, consider changing the callback() signature to accepting the socket via value, allowing the compiler to invoke the move-constructor for you when given an xvalue. Change:

static void callback (boost::asio::ip::tcp::socket && socket)

to:

static void callback (boost::asio::ip::tcp::socket socket)

Overall, the flow of the code is as follows:

void callback(socket&&); // rvalue reference.

void on_accept(acceptor&, socket&, ...) // lvalue reference.
{
  ...
  callback(static_cast<socket&&>(socket)); // Cast to xvalue.
  acceptor.async_accept(socket,
    std::bind(&on_accept, std::ref:acceptor),
      std::ref(socket), // lvalue reference.
      ...);
}

int main()
{
  boost::asio::io_service io_service;
  boost::asio::io_service::work work(io_service);
  boost::asio::ip::tcp::acceptor acceptor(io_service);
  boost::asio::ip::tcp::socket socket(io_service); // Constructor.

  ...

  acceptor.async_accept(socket,
    std::bind(&on_accept, std::ref:acceptor),
      std::ref(socket), // lvalue reference.
      ...);
  io_service.run();
}

Upon successfully accepting the first connection, the socket in main() is open. The on_accept() function invokes callback() with an xvalue, and does not change the state of the socket. Another async_accept() operation is initiated using the already open socket, immediately resulting in the operation's failure. The async_accept() operation fails, invoking on_accept() which will return early, stopping its call chain. As io_service::work is attached to the io_service, execution never returns from io_service::run(), preventing main() from returning and destroying the socket. The final result is no more connections are accepted (no async_accept() operations) and the client is not disconnected (socket is never destroyed).

When callback() changes the state of the socket to close, the issue is no longer present as the pre-condition for async_accept() is met. The other examples meet this pre-condition because:

  • Uncommenting the one line results in the move-constructor being invoking. The moved-from socket will have the same state as if constructed using the socket(io_service&) constructor.
  • The socket is explicitly closed via socket.close().

Upvotes: 2

Related Questions