Search code examples
c++websocketboostasiocrow

Websocket client not closing gracefully


I followed this link to create a websocket client.

This is the class for that:

#include <boost/beast/websocket.hpp>
#include <boost/asio/connect.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/ssl/stream.hpp>
class  WebsocketClient {
public:
    WebsocketClient(const std::string& hostname, const std::string& port, const std::string& url)
        : m_socket(m_io_context)
    {

        boost::asio::ip::tcp::resolver resolver{ m_io_context };
        auto const results = resolver.resolve(hostname, port);
        auto ep = boost::asio::connect(m_socket.next_layer(), results);
        auto host = hostname + std::string(":") + std::to_string(ep.port());
        m_socket.set_option(boost::beast::websocket::stream_base::decorator(
            [](boost::beast::websocket::request_type& req) {
                req.set(boost::beast::http::field::user_agent,
                        std::string(BOOST_BEAST_VERSION_STRING) +
                        " websocket-client-coro");
            }));

        // Perform the websocket handshake
        m_socket.handshake(host, url);
    }

    std::string read() {
        boost::beast::flat_buffer buffer;
        m_socket.read(buffer);
        char* char_ptr = static_cast<char*>(buffer.data().data());
        return std::string(char_ptr, buffer.size());
    }

    void send(const std::string message) {
        m_socket.write(boost::asio::buffer(message));
    }

void close() {

        if (m_socket.is_open()) {
            m_socket.close(boost::beast::websocket::close_code::normal);
        }

        std::cout << "THIS IS NEVER PRINTED " << std::endl;

}

    ~WebsocketClient() {
        close();
        std::cout << "NEVER REACHED " << std::endl;
    }

private:
    boost::asio::io_context m_io_context;
    boost::beast::websocket::stream<boost::asio::ip::tcp::socket> m_socket;
};

I use crowcpp to create a simple websocket server

int main() {
    crow::SimpleApp app;
    CROW_ROUTE(app, "/").websocket()
        .onaccept([](const crow::request& ) {
            return true;
        })
        .onopen([&](crow::websocket::connection& ) {
            std::cout << "CLIENT OPENED - Server "<< std::endl;
        })
        .onclose([&](crow::websocket::connection& , const std::string&) {
            std::cout << "Client CLOSED - Server "<< std::endl;
        });
    std::future<void> m_async_thread_server;

    m_async_thread_server = app
        .bindaddr("127.0.0.1")
        .signal_clear()
        .port(8080)
        .run_async();
     std::this_thread::sleep_for(std::chrono::milliseconds(100));
    auto client = WebsocketClient("127.0.0.1", "8080", "/");

  }

My question is really whether I explicitly invoke client.close() or let the destructor handle it , the problem remains that my thread never returns after m_socket.close(boost::beast::websocket::close_code::normal); By doing a bit of debugging I see that the websocket client is waiting to receive a close header message from the server. By digging a bit into what CrowCpp does after receiving a close request I do see it sends out a close header message.

running the code I get following prints only:

(2023-04-15 12:20:32) [INFO ] Crow/1.0 server is running at http://127.0.0.1:8080 using 2 threads (2023-04-15 12:20:32) [INFO ] Call app.loglevel(crow::LogLevel::Warning) to hide Info level logs.

Client OPENED - Server

Client CLOSED - Server


Solution

  • From the RFC:

    After both sending and receiving a Close message, an endpoint considers the WebSocket connection closed and MUST close the underlying TCP connection. The server MUST close the underlying TCP connection immediately; the client SHOULD wait for the server to close the connection but MAY close the connection at any time after sending and receiving a Close message, e.g., if it has not received a TCP Close from the server in a reasonable time period.

    The Crow server fails to close the connection upon replying to the close frame with its own one.

    It keeps track of close_connection_ when has_recv_close_ as well as has_sent_close_ are true.

    However, only when Crow was the side sending the first close frame (and receiving the responding close frame from the peer), it will actually close the connection. This is apparently just not very well-tested.

    Fixing it

    Starting from v1.0+5 release, edf12f699ec3bf6f751cf73c, I improved the unit test to show the problem, then fixed it, and you can see the problem going away.

    click to see higher quality

    diff --git a/include/crow/websocket.h b/include/crow/websocket.h
    index 4555b70d..037a92c2 100644
    --- a/include/crow/websocket.h
    +++ b/include/crow/websocket.h
    @@ -582,10 +582,14 @@ namespace crow
                               sending_buffers_.clear();
                               if (!ec && !close_connection_)
                               {
    -                              if (!write_buffers_.empty())
    -                                  do_write();
                                   if (has_sent_close_)
    +                              {
                                       close_connection_ = true;
    +                                  adaptor_.close();
    +                                  check_destroy();
    +                              }
    +                              if (!write_buffers_.empty())
    +                                  do_write();
                               }
                               else
                               {
    

    Makes it run fine. The master branch is not compatible with your sample server, but appears to have the same problem still.

    Another problem around closing has been fixed since, though:

    ed87c65c|| Merge pull request #452 from MichaelSB/ws_no_read_after_close
    efee7174|| do not try to read from websocket after sending and receiving the close frame