Boost::Asio server

Question

This is my first time trying to do anything proper in Boost::Asio, so I would love if someone could look at it and tell me if it makes sense. It's based on examples from documentation and bit of googling around, but I want to know if I've put it correctly together.

I know this code doesn't do anything useful. I want to know if foundations are correct before putting more stuff in.

main.cpp

#include <iostream>

#include "server.h"

using namespace std;
using namespace wot;

int main(int, char **) {
    try {
        ios_base::sync_with_stdio(false);

        server s("0.0.0.0", 0xc0fe);
        s.run();

        return 0;
    } catch (exception & e) {
        cerr << "Unhandled exception: " << e.what() << endl;
        return 1;
    }
}

server.h

#ifndef SERVER_H_SFH9W7WB
#define SERVER_H_SFH9W7WB

#include <boost/asio.hpp>

#include "connection.h"
#include "connection_manager.h"

namespace wot {

    class server {
    public:
        server(const server &) =delete;
        server & operator=(const server &) =delete;

        explicit server(const std::string & addr, const size_t port);

        void run();
    private:
        void start_accept();
        void handle_accept(connection::ptr, const boost::system::error_code &);
        void start_await_stop();

        boost::asio::io_service ios_;
        boost::asio::signal_set signals_;
        boost::asio::ip::tcp::acceptor acceptor_;

        connection_manager connection_manager_;
    };

}

#endif /* end of include guard: SERVER_H_SFH9W7WB */

server.cpp

#include "server.h"

#include <iostream>

namespace asio = boost::asio;
namespace ip = boost::asio::ip;

using namespace wot;
using namespace std;

server::server(const std::string & addr, const size_t port)
    : ios_(),
      signals_(ios_),
      acceptor_(ios_)
{
    signals_.add(SIGINT);
    signals_.add(SIGTERM);
#if defined(SIGQUIT)
    signals_.add(SIGQUIT);
#endif

    start_await_stop();

    ip::tcp::endpoint endpoint(ip::address::from_string(addr), port);

    acceptor_.open(endpoint.protocol());
    acceptor_.set_option(ip::tcp::acceptor::reuse_address(true));
    acceptor_.bind(endpoint);
    acceptor_.listen();

    start_accept();
}

void server::run() {
    ios_.run();
}

void server::start_accept() {
    auto new_con = connection::create(
        acceptor_.get_io_service(),
        connection_manager_
    );
    acceptor_.async_accept(
        new_con->socket(),
        bind(&server::handle_accept, this, new_con, placeholders::_1)
    );
}

void server::handle_accept(
    connection::ptr new_con,
    const boost::system::error_code & e
) {
    if (!acceptor_.is_open()) {
        return;
    }
    if (!e) {
        connection_manager_.start(new_con);
    }
    start_accept();
}

void server::start_await_stop() {
    signals_.async_wait(
        [this](boost::system::error_code, int) {
            acceptor_.close();
            connection_manager_.stop_all();
        }
    );
}

connection_manager.h

#ifndef CONNECTION_MANAGER_H_SFH9W7WB
#define CONNECTION_MANAGER_H_SFH9W7WB

#include <set>

#include "connection.h"

namespace wot {

    class connection_manager {
    public:
        connection_manager(const connection_manager &) =delete;
        connection_manager & operator=(const connection_manager &) = delete;

        connection_manager();

        void start(connection::ptr);
        void stop(connection::ptr);
        void stop_all();
    private:
        std::set<connection::ptr> connections_;
    };

}

#endif /* end of include guard: CONNECTION_MANAGER_H_SFH9W7WB */

connection_manager.cpp

#include "connection_manager.h"

using namespace wot;
using namespace std;

connection_manager::connection_manager() {}

void connection_manager::start(connection::ptr c) {
    connections_.insert(c);
    c->start();
}

void connection_manager::stop(connection::ptr c) {
    connections_.erase(c);
    c->stop();
}

void connection_manager::stop_all() {
    for (auto c : connections_) {
        c->stop();
    }
    connections_.clear();
}

connection.h

#ifndef CONNECTION_H_KUDEQ8CR
#define CONNECTION_H_KUDEQ8CR

#include <boost/asio.hpp>

namespace wot {

    class connection_manager;

    class connection : public std::enable_shared_from_this<connection> {
    public:
        using ptr = std::shared_ptr<connection>;

        connection(const connection &) =delete;
        connection & operator=(const connection &) =delete;

        static ptr create(
            boost::asio::io_service &,
            connection_manager &
        );

        void start();
        void stop();
        boost::asio::ip::tcp::socket & socket();
    private:
        explicit connection(
            boost::asio::io_service &,
            connection_manager &
        );

        void start_read();
        void handle_read(const boost::system::error_code &, const size_t);
        void start_write();
        void handle_write(const boost::system::error_code &, const size_t);
        void handle_timeout(const boost::system::error_code &);

        boost::asio::ip::tcp::socket socket_;
        std::array<char, 8192> buffer_;

        connection_manager & connection_manager_;
        boost::asio::deadline_timer timeout_timer_;

        boost::asio::ip::tcp::endpoint remote_endpoint_;
    };

}

#endif /* end of include guard: CONNECTION_H_KUDEQ8CR */

connection.cpp

#include "connection.h"

#include "log.h"
#include "connection_manager.h"

namespace asio = boost::asio;
namespace ip = boost::asio::ip;

using namespace wot;
using namespace std;

connection::ptr connection::create(
    asio::io_service & ios,
    connection_manager & manager
) {
    return ptr(new connection(ios, manager));
}

connection::connection(
    asio::io_service & ios,
    connection_manager & manager
) :
    socket_(ios),
    connection_manager_(manager),
    timeout_timer_(ios)
{}

void connection::start() {
    remote_endpoint_ = socket_.remote_endpoint();

    log_out(remote_endpoint_, "Connection opened by peer.");

    asio::async_write(
        socket_,
        asio::buffer("Hi\n.\n"),
        bind(
            &connection::handle_write,
            shared_from_this(),
            placeholders::_1,
            placeholders::_2
        )
    );

    start_read();
}

void connection::stop() {
    boost::system::error_code ec;
    socket_.shutdown(ip::tcp::socket::shutdown_both, ec);
    if (ec) {
        log_err(
            remote_endpoint_,
            string("Error while shutting down socket: ") + ec.message()
        );
    }
    socket_.close();
    log_out(remote_endpoint_, "Socket closed.");
}

ip::tcp::socket & connection::socket() {
    return socket_;
}

void connection::start_read() {
    timeout_timer_.expires_from_now(boost::posix_time::seconds(5));
    timeout_timer_.async_wait(bind(
        &connection::handle_timeout,
        shared_from_this(),
        placeholders::_1
    ));

    socket_.async_read_some(
        asio::buffer(buffer_),
        bind(
            &connection::handle_read,
            shared_from_this(),
            placeholders::_1,
            placeholders::_2
        )
    );
}

void connection::handle_read(
    const boost::system::error_code & ec,
    const size_t bytes
) {
    if (!ec) {

        start_read();
    } else if (asio::error::eof == ec) {
        log_out(remote_endpoint_, string("Connection closed by peer."));
    } else if (asio::error::operation_aborted != ec) {
        log_out(remote_endpoint_, string("Error: ") + ec.message());
        connection_manager_.stop(shared_from_this());
    }
}

void connection::handle_write(const boost::system::error_code &, size_t) {}

void connection::handle_timeout(const boost::system::error_code & ec) {
    if (!ec) {
        log_out(remote_endpoint_, "Connection timeout.");
        connection_manager_.stop(shared_from_this());
    }
}

/*
void tcp_connection::handle_read(const boost::system::error_code & e, size_t bytes) {
    if (!e) {
        cout << "Bytes: " << bytes << endl;
        cout << "Received: " << string(begin(buffer_), begin(buffer_) + bytes) << endl;
        // append data to received stuff
        std::copy(begin(buffer_), begin(buffer_) + bytes, std::back_inserter(data_));
        auto terminator = std::search(
            begin(data_),
            end(data_),
            msg_terminator,
            msg_terminator + msg_terminator_len
        );
        if (terminator != end(data_)) {
            string msg = string(data_.begin(), terminator + msg_terminator_len);
            log_out(string("Received message: ") + msg);
            data_.erase(data_.begin(), terminator + msg_terminator_len);
        } else {
            cout << "WMessage: \"" << std::string(data_.begin(), terminator) << "\"" << endl;
        }
        cout << socket_.remote_endpoint().address().to_string() << ": Read done." << endl;
        cout << "Data: \"" << std::string(data_.data()) << "\"" << endl;
        start_read();
    } else if (e == boost::asio::error::eof) {
        log_out("Connection closed cleanly by peer.");
    } else {
        cerr << "Error: " << e.message() << endl;
        cerr << "Bytes: " << bytes << endl;
        cerr << static_cast<int>(buffer_[0]) << " " << static_cast<int>(buffer_[1]) << endl;
    }
}
}
*/

log.h

#ifndef LOG_H
#define LOG_H

#include <string>

#include <boost/asio.hpp>

namespace wot {

    void log_out(const std::string &);
    void log_out(const boost::asio::ip::tcp::endpoint &, const std::string &);

    void log_err(const std::string &);
    void log_err(const boost::asio::ip::tcp::endpoint &, const std::string &);

}

#endif

log.cpp

#include "log.h"

#include <ctime>
#include <iostream>

using namespace wot;
using namespace std;

namespace {
    void log(ostream & os, const string & msg) {
        time_t now;
        time(&now);
        char buf[sizeof "2011-10-08 07:07:09"];
        strftime(buf, sizeof buf, "%F %T", gmtime(&now));

        os << buf << ": " << msg << endl;
    }
}

void wot::log_out(const string & msg) {
    log(cout, msg);
}

void wot::log_out(
    const boost::asio::ip::tcp::endpoint & endpoint,
    const string & msg
) {
    log_out(endpoint.address().to_string() + ": " + msg);
}

void wot::log_err(const string & msg) {
    log(cerr, msg);
}

void wot::log_err(
    const boost::asio::ip::tcp::endpoint & endpoint,
    const string & msg
) {
    log_err(endpoint.address().to_string() + ": " + msg);
}

I guess I'm mainly interested in two things:

1. Does this code do what I think it does (listen for connections, kill connections in case nothing happened for 5 sec and don't leak any memory)?
2. It is more-or-less how is Asio supposed to be used? Or should I do something differently?

Answer 1

I see a number of things that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every module in every program is a bad habit that you'd do well to avoid.

Reconsider the complex include guards

When I see something like this:

#ifndef SERVER_H_SFH9W7WB

I wonder, "what is the significance of SFH9W7WB?" I am guessing it's an autogenerated suffix to try to avoid name clashes, but I find it distracting and it is, in this code, also not needed since there are no name clashes.

Reconsider embedding the io_service in the server object

The current code has one io_service object per server, but that's not really necessary. A single io_service can serve multiple server objects. If you're worried about multithreading, reassure yourself by reading this: http://www.boost.org/doc/libs/1_61_0/doc/html/boost_asio/overview/core/threads.html

Prefer lambdas to boost::placeholders

Instead of writing something like this:

acceptor_.async_accept(
    new_con->socket(),
    bind(&server::handle_accept, this, new_con, placeholders::_1)
);

You could instead use a lambda, and perhaps eliminate handle_except entirely. See this page of code diffs to see what's different when using C++11 and lambdas.

Provide a means to gracefully exit the program

Other than killing the process, there's not a way to gracefully exit the program. One way I've done this in the past is to use something like this

int main() {
    std::thread myServer{serve};
    std::string command;
    std::cout << "Enter the word \"quit\" to exit the program and shut down the server\n";
    while (!done && std::cin >> command) {
        if (command == "quit") {
            done = true;
        }
    }
    myServer.join();
}

The serve function in this case is just a simple function to run the server and monitor the static boolean variable done.

Consider making a log object

Rather than having four discrete functions for the log functions, consider making a log object which could store a reference to the desired stream. This would then also allow for very simple logging to a file without having to change a lot of code.

Let the compiler generate code

Instead of including an empty constructor like this:

connection_manager::connection_manager() {}

Let the compiler automatically generate the code instead. If you want to make it very explicit that you're using the default constructor, you can put this in the declaration:

connection_manager() = default;

Don't use sync_with_stdio unless needed

It's not technically wrong, but as was already mentioned in the comments, use of sync_with_stdio within main doesn't really appear to be useful, and therefore should be omitted.

General advice

If you're just starting to learn asio, I would strongly recommend working through the tutorial, one piece at a time, until you really understand how the pieces all fit together. I'd also highly recommend the book C++ Concurrency in Action by Anthony Williams. There are a number of useful constructs in C++ that allow for multithreading (e.g. the shared_from_this already used within your code) but it's also very easy to get it slightly wrong and introduce a hard-to-find bug that only manifests itself occasionally (e.g. specifying a callback that's a member function for an object that may be deleted by the time the callback is invoked). I don't see such an error in your code, but I also didn't invest the time required to find such an error, should it exist.

Generally speaking, I'd recommend thinking carefully about when objects are created and destroyed. I often find that a little instrumentation in the code can help a lot with verifying your understanding.