From a0326b3f2b7bee8d4cc5675e14175fd97dfedfe9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 24 Sep 2013 23:17:53 +0900 Subject: [PATCH] nghttpx: Handle error from bufferevent_socket_new and event_base_new --- src/shrpx.cc | 8 +++++-- src/shrpx_http_downstream_connection.cc | 6 ++++- src/shrpx_listen_handler.cc | 11 ++++++++++ src/shrpx_spdy_session.cc | 17 +++++++++++++++ src/shrpx_ssl.cc | 11 ++++++++-- src/shrpx_worker.cc | 29 ++++++++++++++++++------- 6 files changed, 69 insertions(+), 13 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index d72cd13b..d092fb2d 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -59,7 +59,7 @@ namespace { void ssl_acceptcb(evconnlistener *listener, int fd, sockaddr *addr, int addrlen, void *arg) { - ListenHandler *handler = reinterpret_cast(arg); + auto handler = reinterpret_cast(arg); handler->accept_connection(fd, addr, addrlen); } } // namespace @@ -200,7 +200,7 @@ evconnlistener* create_evlistener(ListenHandler *handler, int family) return 0; } - evconnlistener *evlistener = evconnlistener_new + auto evlistener = evconnlistener_new (handler->get_evbase(), ssl_acceptcb, handler, @@ -249,6 +249,10 @@ namespace { int event_loop() { auto evbase = event_base_new(); + if(!evbase) { + LOG(FATAL) << "event_base_new() failed"; + exit(EXIT_FAILURE); + } SSL_CTX *sv_ssl_ctx, *cl_ssl_ctx; if(get_config()->client_mode) { diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 337ccc63..8264efb6 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -74,6 +74,10 @@ int HttpDownstreamConnection::attach_downstream(Downstream *downstream) bev_ = bufferevent_socket_new (evbase, -1, BEV_OPT_CLOSE_ON_FREE | BEV_OPT_DEFER_CALLBACKS); + if(!bev_) { + DCLOG(INFO, this) << "bufferevent_socket_new() failed"; + return SHRPX_ERR_NETWORK; + } int rv = bufferevent_socket_connect (bev_, // TODO maybe not thread-safe? @@ -81,7 +85,7 @@ int HttpDownstreamConnection::attach_downstream(Downstream *downstream) get_config()->downstream_addrlen); if(rv != 0) { bufferevent_free(bev_); - bev_ = 0; + bev_ = nullptr; return SHRPX_ERR_NETWORK; } if(LOG_ENABLED(INFO)) { diff --git a/src/shrpx_listen_handler.cc b/src/shrpx_listen_handler.cc index 853cf78e..ff4b673c 100644 --- a/src/shrpx_listen_handler.cc +++ b/src/shrpx_listen_handler.cc @@ -82,6 +82,13 @@ void ListenHandler::create_worker_thread(size_t num) } auto bev = bufferevent_socket_new(evbase_, info->sv[0], BEV_OPT_DEFER_CALLBACKS); + if(!bev) { + LLOG(ERROR, this) << "bufferevent_socket_new() failed"; + for(size_t j = 0; j < 2; ++j) { + close(info->sv[j]); + } + continue; + } info->bev = bev; if(LOG_ENABLED(INFO)) { LLOG(INFO, this) << "Created thread #" << num_worker_; @@ -99,6 +106,10 @@ int ListenHandler::accept_connection(evutil_socket_t fd, if(num_worker_ == 0) { auto client = ssl::accept_connection(evbase_, sv_ssl_ctx_, fd, addr, addrlen); + if(!client) { + LLOG(ERROR, this) << "ClientHandler creation failed"; + return 0; + } client->set_spdy_session(spdy_); } else { size_t idx = worker_round_robin_cnt_ % num_worker_; diff --git a/src/shrpx_spdy_session.cc b/src/shrpx_spdy_session.cc index a2f4d104..cec3e25f 100644 --- a/src/shrpx_spdy_session.cc +++ b/src/shrpx_spdy_session.cc @@ -363,6 +363,10 @@ int SpdySession::initiate_connection() << get_config()->downstream_http_proxy_port; } bev_ = bufferevent_socket_new(evbase_, -1, BEV_OPT_DEFER_CALLBACKS); + if(!bev_) { + SSLOG(ERROR, this) << "bufferevent_socket_new() failed"; + return SHRPX_ERR_NETWORK; + } bufferevent_enable(bev_, EV_READ); bufferevent_set_timeouts(bev_, &get_config()->downstream_read_timeout, &get_config()->downstream_write_timeout); @@ -419,6 +423,11 @@ int SpdySession::initiate_connection() bev_ = bufferevent_openssl_socket_new(evbase_, fd_, ssl_, BUFFEREVENT_SSL_CONNECTING, BEV_OPT_DEFER_CALLBACKS); + if(!bev_) { + SSLOG(ERROR, this) << "bufferevent_socket_new() failed"; + SSL_free(ssl_); + return SHRPX_ERR_NETWORK; + } rv = bufferevent_socket_connect (bev_, // TODO maybe not thread-safe? @@ -427,6 +436,10 @@ int SpdySession::initiate_connection() } else if(state_ == DISCONNECTED) { // Without TLS and proxy. bev_ = bufferevent_socket_new(evbase_, -1, BEV_OPT_DEFER_CALLBACKS); + if(!bev_) { + SSLOG(ERROR, this) << "bufferevent_socket_new() failed"; + return SHRPX_ERR_NETWORK; + } rv = bufferevent_socket_connect (bev_, const_cast(&get_config()->downstream_addr.sa), @@ -435,6 +448,10 @@ int SpdySession::initiate_connection() assert(state_ == PROXY_CONNECTED); // Without TLS but with proxy. bev_ = bufferevent_socket_new(evbase_, fd_, BEV_OPT_DEFER_CALLBACKS); + if(!bev_) { + SSLOG(ERROR, this) << "bufferevent_socket_new() failed"; + return SHRPX_ERR_NETWORK; + } // Connection already established. eventcb(bev_, BEV_EVENT_CONNECTED, this); // eventcb() has no return value. Check state_ to whether it was diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index 4e5eb56e..b18d5b18 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -312,7 +312,7 @@ ClientHandler* accept_connection(event_base *evbase, SSL_CTX *ssl_ctx, if(!ssl) { LOG(ERROR) << "SSL_new() failed: " << ERR_error_string(ERR_get_error(), nullptr); - return 0; + return nullptr; } bev = bufferevent_openssl_socket_new (evbase, fd, ssl, @@ -320,10 +320,17 @@ ClientHandler* accept_connection(event_base *evbase, SSL_CTX *ssl_ctx, } else { bev = bufferevent_socket_new(evbase, fd, BEV_OPT_DEFER_CALLBACKS); } + if(!bev) { + LOG(ERROR) << "bufferevent_socket_new() failed"; + if(ssl) { + SSL_free(ssl); + } + return nullptr; + } return new ClientHandler(bev, fd, ssl, host); } else { LOG(ERROR) << "getnameinfo() failed: " << gai_strerror(rv); - return 0; + return nullptr; } } diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index c9890480..f377298f 100644 --- a/src/shrpx_worker.cc +++ b/src/shrpx_worker.cc @@ -26,6 +26,7 @@ #include #include + #include #include @@ -75,20 +76,32 @@ void eventcb(bufferevent *bev, short events, void *arg) void Worker::run() { - auto evbase = event_base_new(); - auto bev = bufferevent_socket_new(evbase, fd_, BEV_OPT_DEFER_CALLBACKS); - SpdySession *spdy = nullptr; + auto evbase = std::unique_ptr + (event_base_new(), event_base_free); + if(!evbase) { + LOG(ERROR) << "event_base_new() failed"; + return; + } + auto bev = std::unique_ptr + (bufferevent_socket_new(evbase.get(), fd_, BEV_OPT_DEFER_CALLBACKS), + bufferevent_free); + if(!bev) { + LOG(ERROR) << "bufferevent_socket_new() failed"; + return; + } + std::unique_ptr spdy; if(get_config()->downstream_proto == PROTO_SPDY) { - spdy = new SpdySession(evbase, cl_ssl_ctx_); + spdy = util::make_unique(evbase.get(), cl_ssl_ctx_); if(spdy->init_notification() == -1) { DIE(); } } - auto receiver = util::make_unique(sv_ssl_ctx_, spdy); - bufferevent_enable(bev, EV_READ); - bufferevent_setcb(bev, readcb, 0, eventcb, receiver.get()); + auto receiver = util::make_unique(sv_ssl_ctx_, + spdy.get()); + bufferevent_enable(bev.get(), EV_READ); + bufferevent_setcb(bev.get(), readcb, nullptr, eventcb, receiver.get()); - event_base_loop(evbase, 0); + event_base_loop(evbase.get(), 0); } void start_threaded_worker(WorkerInfo *info)