Crow icon indicating copy to clipboard operation
Crow copied to clipboard

[bad] throw in ctor if TCP port is already occupied

Open z16166 opened this issue 3 years ago • 4 comments

If TCP port is already occupied, the constructor of "crow::Server" will throw exception by bind() of boost:asio(I'm using boost::asio). It's a bad practice to throw in ctor. Please refer to the following line: acceptor_(io_service_, tcp::endpoint(boost::asio::ip::address::from_string(bindaddr), port)),

namespace crow
{
    using namespace boost;
    using tcp = asio::ip::tcp;

    template<typename Handler, typename Adaptor = SocketAdaptor, typename... Middlewares>
    class Server
    {
    public:
        Server(Handler* handler, std::string bindaddr, uint16_t port, std::string server_name = std::string("Crow/") + VERSION, std::tuple<Middlewares...>* middlewares = nullptr, uint16_t concurrency = 1, uint8_t timeout = 5, typename Adaptor::context* adaptor_ctx = nullptr):
          acceptor_(io_service_, tcp::endpoint(boost::asio::ip::address::from_string(bindaddr), port)),
          signals_(io_service_),

z16166 avatar Aug 02 '22 07:08 z16166

Thank you for pointing this out. What would you suggest to resolve it?

The-EDev avatar Aug 02 '22 07:08 The-EDev

we can construct acceptor obj without ip/port in crow::Server ctor, then call its open()/bind()/listen() method later in crow::Server::run(). And we also need to propogate the bind( ) failure back to caller threads.

The simplest way is to move the construction of acceptor_ to the very end of constructor. But this can't propogate the bind( ) failure back to caller threads if we use async run.

namespace crow
{
    using namespace boost;
    using tcp = asio::ip::tcp;

    template<typename Handler, typename Adaptor = SocketAdaptor, typename... Middlewares>
    class Server
    {
    public:
        Server(Handler* handler, std::string bindaddr, uint16_t port, ...):
          acceptor_(io_service_){ 
          }

        void run() {
            const auto endpoint = tcp::endpoint(
                boost::asio::ip::address::from_string(bindaddr_), port_);
            acceptor_.open(endpoint.protocol());
            acceptor_.set_option(
                boost::asio::ip::tcp::acceptor::reuse_address(true));
            acceptor_.bind(endpoint);
            acceptor_.listen();
        }

z16166 avatar Aug 02 '22 07:08 z16166

My suggestion updated as above. Thanks !

z16166 avatar Aug 02 '22 13:08 z16166

new suggestion: use SO_EXCLUSIVEADDRUSE instead of SO_REUSEADDR on Windows.

reason:
https://docs.microsoft.com/en-us/windows/win32/winsock/using-so-reuseaddr-and-so-exclusiveaddruse

solution(copied from https://stackoverflow.com/questions/7164879/boost-asio-why-dont-i-get-bind-address-already-in-use-in-windows-but-do-ge):

typedef boost::asio::detail::socket_option::boolean<BOOST_ASIO_OS_DEF(SOL_SOCKET), SO_EXCLUSIVEADDRUSE> excluse_address;
acceptor_.set_option(excluse_address(true));

z16166 avatar Sep 14 '22 14:09 z16166

@z16166 it seems you closed this issue along with #571, #572, #573, and #574. If I'm not wrong they've not been resolved yet..

The-EDev avatar Dec 10 '22 10:12 The-EDev