Avoiding global variables making boost asio socket wrapper

Question

i have this socket-tcp.h with an wrapper of socket implementation using boost asio:

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

static const std::string PORT = "65432";  
static const std::string HOST = "127.0.0.1";  

struct Client
{
    boost::asio::io_service& io_service;
    boost::asio::ip::tcp::socket socket;
    
    Client(boost::asio::io_service& svc, std::string const& host, std::string const& port) 
        : io_service(svc), socket(io_service) 
    {
        boost::asio::ip::tcp::resolver resolver(io_service);
        boost::asio::ip::tcp::resolver::iterator endpoint = resolver.resolve(boost::asio::ip::tcp::resolver::query(host, port));
        boost::asio::connect(this->socket, endpoint);
    };

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


boost::asio::io_service svc;
Client client(svc, HOST, PORT);

class TcpSocket
{

    private:
        std::string HOST; 
        std::string PORT;
        
    public:
        
        TcpSocket (std::string const& host, std::string const& port): HOST{ host },PORT{ port }
        {
        }

        void send(std::string const& message)
        {
            boost::thread t(client_thread,boost::ref(message),boost::ref(HOST),boost::ref(PORT));
            t.join();
        }

        static void client_thread(std::string const& message,std::string const& host,std::string const& port) 
        {
            client.send(message); 
        }
            
};

so that my main file looks like:

#include "socket-tcp.h"

int main()
{
    TcpSocket socket(PORT,HOST);
    std::string message = "socket implemented using global variables";
    while (true)
    {
        socket.send(message);
    }
}

I'm trying to figure out a way of implement this without the global variables

boost::asio::io_service svc;
Client client(svc, HOST, PORT);

such that TcpSocket be like:

class TcpSocket
{

    //Object* myObject; // Will not try to call the constructor or do any initializing
    //myObject = new Object(...);  // Initialised now
    private:
        std::string HOST; 
        std::string PORT;
        boost::asio::io_service svc;
        
    public:
        
        TcpSocket (std::string const& host, std::string const& port): HOST{ host },PORT{ port }
        {
            Client client(svc, HOST, PORT);
        }

        void send(std::string const& message)
        {
            boost::thread t(client_thread,boost::ref(message),boost::ref(HOST),boost::ref(PORT));
            t.join();
        }

        static void client_thread(std::string const& message,std::string const& host,std::string const& port) 
        {
            client.send(message); 
        }
            
};

but i end up with the runtime error:

terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::system::system_error> >'
  what():  resolve: Service not found

is there a way to avoid using these global variables (objects), and keeping the same socket open all the time, without closing and opening it again at each new message?

I accept better implementations or suggestions for this wrapper, but the goal is to keep main as simple and clear as possible.

sehe · Accepted Answer · 2021-09-10T20:46:46.160

The "service not found" merely means that the port is not a valid service. That's because you swapped host and port parameters.

But that is the least of your worries.

You're playing fast and loose with object lifetimes. For example, you pass message all the way to another thread &&by reference** (std::string const&), but the object referenced lives on the stack (as a function argument) so that invokes Undefined Behaviour.

Besides, it's not clear how the thread (or client_thread) is supposed to access the Client instance (that your code constructs as a local variable in the TcpSocket constructor only). Let alone that it would be unsafe to share that client across threads.

Medium Fix

Live On Coliru

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

static const std::string HOST = "127.0.0.1";  
static const std::string PORT = "65432";  

using boost::asio::ip::tcp;

struct Client
{
    tcp::socket socket;

    //template <typename Executor>
    using Executor = boost::asio::io_service&;
    Client(Executor executor, std::string const& host, std::string const& port)
        : socket(executor)
    {
        std::cout << std::quoted(host) << std::endl;
        std::cout << std::quoted(port) << std::endl;
        auto ep = tcp::resolver{socket.get_executor()}.resolve(host, port);
        connect(socket, ep);
    };

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

class TcpSocket {
  private:
    std::string             _host;
    std::string             _port;
    boost::asio::io_service _svc;

  public:
    TcpSocket(std::string const& host, std::string const& port)
        : _host{host}
        , _port{port}
    {
    }

    void send(std::string const& message)
    {
        boost::thread t( //
            client_thread, boost::ref(_svc), message, _host, _port);
        t.join();
    }

    static void client_thread( //
        boost::asio::io_service& svc,
        std::string              message, // NOT A REFERENCE
        std::string              host,    // same
        std::string              port)
    {
        Client client(svc, host, port);
        client.send(message);
    }
};

//#include "socket-tcp.h"

int main()
{
    TcpSocket   socket(HOST, PORT);
    std::string const message = "socket implemented using global variables";
    while (true) {
        socket.send(message);
    }
}

More Notes

The thread is by definition useless, since you join it immediately. But if you really wanted that, please consider c++11 style:

void send(std::string const& message)
{
    boost::thread t([=, &_svc] {
        Client client(_svc, _host, _port);
        client.send(message);
    });
    t.join();
}

It makes far more sense to do it on the main thread:

void send(std::string const& message)
{
    Client client(_svc, _host, _port);
    client.send(message);
}

Or to use async IO. Note that you should probably prefer to pass executors rather than sharing references to an execution context. Also, prefer io_context because io_service is deprecated.

Here's a simplified program that still does the same: Live On Coliru

BONUS - Async With A Thread Pool

Seems like you wanted the messages to be delivered in async fashion. However, you didn't know how to get threads in the background. Regardless, it wouldn't be a safe idea to "just create a new thread" each time. The professional approach is to use a thread pool.

This example uses a ddefault boost::asio::thread_pool and throws in a strand (even though it's not currently required) and a Session object to hold all the state for a single message. Note that we now cache the resolver results. That can be a good thing ,or a bad thing depending on your application.

Live On Coliru

#include <boost/asio.hpp>
#include <iostream>
#include <boost/thread.hpp>
#include <boost/enable_shared_from_this.hpp>

using boost::asio::ip::tcp;
using boost::system::error_code;

class TcpClient {
  public:
    TcpClient(std::string const& host, std::string const& port)
        : _ep(tcp::resolver{_io}.resolve(host, port))
    { }

    void send(std::string message)
    {
        boost::make_shared<Session>( //
            make_strand(_io.get_executor()), std::move(message), _ep)
            ->run();
    }

    ~TcpClient() {
        //_io.stop(); // optionally
        _io.join();
    }

  private:
    boost::asio::thread_pool _io;
    tcp::resolver::results_type _ep ;

    struct Session : boost::enable_shared_from_this<Session> {
        Session(auto executor, std::string message,
                tcp::resolver::results_type ep)
            : _socket(executor)
            , _message(std::move(message))
            , _ep(ep)
        {
        }

        void run() {
            async_connect( //
                _socket, _ep,
                [this, self = shared_from_this()](error_code ec,
                                                  tcp::endpoint) {
                    async_write(_socket, boost::asio::buffer(_message),
                                [this, self](error_code ec, size_t) {});
                });
        }

        tcp::socket                 _socket;
        std::string                 _message;
        tcp::resolver::results_type _ep;
    };
};

//#include "socket-tcp.h"

int main()
{
    TcpClient socket("127.0.0.1", "65432");
    for (int i = 0; i<100; ++i) {
        socket.send("socket implemented using async IO on thread pool " +
                    std::to_string(i) + "\n");
    }
}

Added a version with a threadpool for good measure http://coliru.stacked-crooked.com/ — sehe, Sep 10 '21 at 20:47

Avoiding global variables making boost asio socket wrapper

1 Answers1

Medium Fix

More Notes

BONUS - Async With A Thread Pool