diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index 97c59fff..e5989fe2 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -462,12 +462,13 @@ void ClientHandler::set_should_close_after_write(bool f) should_close_after_write_ = f; } -void ClientHandler::pool_downstream_connection(DownstreamConnection *dconn) +void ClientHandler::pool_downstream_connection +(std::unique_ptr dconn) { if(LOG_ENABLED(INFO)) { - CLOG(INFO, this) << "Pooling downstream connection DCONN:" << dconn; + CLOG(INFO, this) << "Pooling downstream connection DCONN:" << dconn.get(); } - dconn_pool_.insert(dconn); + dconn_pool_.insert(dconn.release()); } void ClientHandler::remove_downstream_connection(DownstreamConnection *dconn) @@ -477,9 +478,11 @@ void ClientHandler::remove_downstream_connection(DownstreamConnection *dconn) << " from pool"; } dconn_pool_.erase(dconn); + delete dconn; } -DownstreamConnection* ClientHandler::get_downstream_connection() +std::unique_ptr +ClientHandler::get_downstream_connection() { if(dconn_pool_.empty()) { if(LOG_ENABLED(INFO)) { @@ -487,19 +490,20 @@ DownstreamConnection* ClientHandler::get_downstream_connection() << " Create new one"; } if(http2session_) { - return new Http2DownstreamConnection(this); + return util::make_unique(this); } else { - return new HttpDownstreamConnection(this); + return util::make_unique(this); } - } else { - auto dconn = *std::begin(dconn_pool_); - dconn_pool_.erase(dconn); - if(LOG_ENABLED(INFO)) { - CLOG(INFO, this) << "Reuse downstream connection DCONN:" << dconn - << " from pool"; - } - return dconn; } + + auto dconn = std::unique_ptr(*std::begin(dconn_pool_)); + dconn_pool_.erase(dconn.get()); + if(LOG_ENABLED(INFO)) { + CLOG(INFO, this) << "Reuse downstream connection DCONN:" << dconn.get() + << " from pool"; + } + + return dconn; } size_t ClientHandler::get_outbuf_length() diff --git a/src/shrpx_client_handler.h b/src/shrpx_client_handler.h index 3fd9f490..5cbe2955 100644 --- a/src/shrpx_client_handler.h +++ b/src/shrpx_client_handler.h @@ -64,9 +64,9 @@ public: void set_should_close_after_write(bool f); Upstream* get_upstream(); - void pool_downstream_connection(DownstreamConnection *dconn); + void pool_downstream_connection(std::unique_ptr dconn); void remove_downstream_connection(DownstreamConnection *dconn); - DownstreamConnection* get_downstream_connection(); + std::unique_ptr get_downstream_connection(); size_t get_outbuf_length(); SSL* get_ssl() const; void set_http2_session(Http2Session *http2session); diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 0e28269d..771abd65 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -42,7 +42,6 @@ Downstream::Downstream(Upstream *upstream, int32_t stream_id, int32_t priority) : request_bodylen_(0), response_bodylen_(0), upstream_(upstream), - dconn_(nullptr), response_body_buf_(nullptr), upstream_rtimerev_(nullptr), upstream_wtimerev_(nullptr), @@ -98,22 +97,50 @@ Downstream::~Downstream() if(downstream_wtimerev_) { event_free(downstream_wtimerev_); } - if(dconn_) { - delete dconn_; - } if(LOG_ENABLED(INFO)) { DLOG(INFO, this) << "Deleted"; } } -void Downstream::set_downstream_connection(DownstreamConnection *dconn) +int Downstream::attach_downstream_connection +(std::unique_ptr dconn) { - dconn_ = dconn; + if(dconn->attach_downstream(this) != 0) { + return -1; + } + + dconn_ = std::move(dconn); + + return 0; +} + +void Downstream::detach_downstream_connection() +{ + if(!dconn_) { + return; + } + + dconn_->detach_downstream(this); + + auto handler = dconn_->get_client_handler(); + + handler->pool_downstream_connection + (std::unique_ptr(dconn_.release())); +} + +void Downstream::release_downstream_connection() +{ + dconn_.release(); } DownstreamConnection* Downstream::get_downstream_connection() { - return dconn_; + return dconn_.get(); +} + +std::unique_ptr Downstream::pop_downstream_connection() +{ + return std::unique_ptr(dconn_.release()); } void Downstream::pause_read(IOCtrlReason reason) diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 4fe9b305..98581d25 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -31,6 +31,7 @@ #include #include +#include #include #include @@ -64,8 +65,15 @@ public: void set_downstream_stream_id(int32_t stream_id); int32_t get_downstream_stream_id() const; - void set_downstream_connection(DownstreamConnection *dconn); + int attach_downstream_connection + (std::unique_ptr dconn); + void detach_downstream_connection(); + // Releases dconn_, without freeing it. + void release_downstream_connection(); DownstreamConnection* get_downstream_connection(); + // Returns dconn_ and nullifies dconn_. + std::unique_ptr pop_downstream_connection(); + // Returns true if output buffer is full. If underlying dconn_ is // NULL, this function always returns false. bool get_output_buffer_full(); @@ -277,7 +285,7 @@ private: int64_t response_bodylen_; Upstream *upstream_; - DownstreamConnection *dconn_; + std::unique_ptr dconn_; // This buffer is used to temporarily store downstream response // body. nghttp2 library reads data from this in the callback. evbuffer *response_body_buf_; diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index ea2d2558..a8a43f4a 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -84,7 +84,7 @@ Http2DownstreamConnection::~Http2DownstreamConnection() // Downstream and DownstreamConnection may be deleted // asynchronously. if(downstream_) { - downstream_->set_downstream_connection(nullptr); + downstream_->release_downstream_connection(); } if(LOG_ENABLED(INFO)) { DCLOG(INFO, this) << "Deleted"; @@ -121,7 +121,7 @@ int Http2DownstreamConnection::attach_downstream(Downstream *downstream) if(http2session_->get_state() == Http2Session::DISCONNECTED) { http2session_->notify(); } - downstream->set_downstream_connection(this); + downstream_ = downstream; downstream_->init_downstream_timer(); @@ -147,12 +147,9 @@ void Http2DownstreamConnection::detach_downstream(Downstream *downstream) http2session_->notify(); } - downstream->set_downstream_connection(nullptr); downstream->disable_downstream_rtimer(); downstream->disable_downstream_wtimer(); downstream_ = nullptr; - - client_handler_->pool_downstream_connection(this); } int Http2DownstreamConnection::submit_rst_stream(Downstream *downstream, diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 22322c6a..de21af55 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -85,11 +85,7 @@ int on_stream_close_callback if(!downstream->get_upgraded() && !downstream->get_response_connection_close()) { // Keep-alive - auto dconn = downstream->get_downstream_connection(); - - if(dconn) { - dconn->detach_downstream(downstream); - } + downstream->detach_downstream_connection(); } upstream->remove_downstream(downstream); @@ -399,8 +395,8 @@ void Http2Upstream::initiate_downstream(std::unique_ptr downstream) { int rv; - auto dconn = handler_->get_downstream_connection(); - rv = dconn->attach_downstream(downstream.get()); + rv = downstream->attach_downstream_connection + (handler_->get_downstream_connection()); if(rv != 0) { // downstream connection fails, send error page if(error_reply(downstream.get(), 503) != 0) { @@ -849,8 +845,8 @@ void downstream_readcb(bufferevent *bev, void *ptr) // on_stream_close_callback. upstream->rst_stream(downstream, infer_upstream_rst_stream_error_code (downstream->get_response_rst_stream_error_code())); - downstream->set_downstream_connection(nullptr); - delete dconn; + downstream->pop_downstream_connection(); + // dconn was deleted dconn = nullptr; } else { auto rv = downstream->on_read(); @@ -870,8 +866,8 @@ void downstream_readcb(bufferevent *bev, void *ptr) downstream->set_response_state(Downstream::MSG_COMPLETE); // Clearly, we have to close downstream connection on http parser // failure. - downstream->set_downstream_connection(nullptr); - delete dconn; + downstream->pop_downstream_connection(); + // dconn was deleted dconn = nullptr; } } @@ -933,8 +929,8 @@ void downstream_eventcb(bufferevent *bev, short events, void *ptr) // Delete downstream connection. If we don't delete it here, it // will be pooled in on_stream_close_callback. - downstream->set_downstream_connection(nullptr); - delete dconn; + downstream->pop_downstream_connection(); + // dconn was deleted dconn = nullptr; // downstream wil be deleted in on_stream_close_callback. if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) { @@ -989,8 +985,8 @@ void downstream_eventcb(bufferevent *bev, short events, void *ptr) // Delete downstream connection. If we don't delete it here, it // will be pooled in on_stream_close_callback. - downstream->set_downstream_connection(nullptr); - delete dconn; + downstream->pop_downstream_connection(); + // dconn was deleted dconn = nullptr; if(downstream->get_response_state() == Downstream::MSG_COMPLETE) { diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 3e73dc19..56cb9db1 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -65,7 +65,7 @@ HttpDownstreamConnection::~HttpDownstreamConnection() // Downstream and DownstreamConnection may be deleted // asynchronously. if(downstream_) { - downstream_->set_downstream_connection(nullptr); + downstream_->release_downstream_connection(); } } @@ -104,7 +104,7 @@ int HttpDownstreamConnection::attach_downstream(Downstream *downstream) DCLOG(INFO, this) << "Connecting to downstream server"; } } - downstream->set_downstream_connection(this); + downstream_ = downstream; ioctrl_.set_bev(bev_); @@ -358,7 +358,7 @@ void idle_eventcb(bufferevent *bev, short events, void *arg) } auto client_handler = dconn->get_client_handler(); client_handler->remove_downstream_connection(dconn); - delete dconn; + // dconn was deleted } } // namespace @@ -367,8 +367,7 @@ void HttpDownstreamConnection::detach_downstream(Downstream *downstream) if(LOG_ENABLED(INFO)) { DCLOG(INFO, this) << "Detaching from DOWNSTREAM:" << downstream; } - downstream->set_downstream_connection(0); - downstream_ = 0; + downstream_ = nullptr; ioctrl_.force_resume_read(); bufferevent_enable(bev_, EV_READ); bufferevent_setcb(bev_, 0, 0, idle_eventcb, this); @@ -377,7 +376,6 @@ void HttpDownstreamConnection::detach_downstream(Downstream *downstream) bufferevent_set_timeouts(bev_, &get_config()->downstream_idle_read_timeout, &get_config()->downstream_write_timeout); - client_handler_->pool_downstream_connection(this); } bufferevent* HttpDownstreamConnection::get_bev() diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index dc0a2a15..61696f76 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -184,14 +184,12 @@ int htp_hdrs_completecb(http_parser *htp) } } - auto dconn = upstream->get_client_handler()->get_downstream_connection(); - - rv = dconn->attach_downstream(downstream); + rv = downstream->attach_downstream_connection + (upstream->get_client_handler()->get_downstream_connection()); if(rv != 0) { downstream->set_request_state(Downstream::CONNECT_FAIL); - downstream->set_downstream_connection(nullptr); - delete dconn; + return -1; } @@ -405,17 +403,13 @@ int HttpsUpstream::on_write() // We need to postpone detachment until all data are sent so that // we can notify nghttp2 library all data consumed. if(downstream->get_response_state() == Downstream::MSG_COMPLETE) { - auto dconn = downstream->get_downstream_connection(); - if(downstream->get_response_connection_close()) { - // Connection close - downstream->set_downstream_connection(nullptr); - - delete dconn; + downstream->pop_downstream_connection(); + // dconn was deleted } else { // Keep-alive - dconn->detach_downstream(downstream); + downstream->detach_downstream_connection(); } } @@ -511,14 +505,12 @@ void https_downstream_readcb(bufferevent *bev, void *ptr) if(handler->get_outbuf_length() == 0) { if(downstream->get_response_connection_close()) { // Connection close - downstream->set_downstream_connection(nullptr); - - delete dconn; + downstream->pop_downstream_connection(); dconn = nullptr; } else { // Keep-alive - dconn->detach_downstream(downstream); + downstream->detach_downstream_connection(); } } diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 2f7cad7d..fd8515e7 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -120,10 +120,7 @@ void on_stream_close_callback if(!downstream->get_upgraded() && !downstream->get_response_connection_close()) { // Keep-alive - auto dconn = downstream->get_downstream_connection(); - if(dconn) { - dconn->detach_downstream(downstream); - } + downstream->detach_downstream_connection(); } upstream->remove_downstream(downstream); // downstrea was deleted @@ -265,8 +262,8 @@ void SpdyUpstream::maintain_downstream_concurrency() void SpdyUpstream::initiate_downstream(std::unique_ptr downstream) { - auto dconn = handler_->get_downstream_connection(); - int rv = dconn->attach_downstream(downstream.get()); + int rv = downstream->attach_downstream_connection + (handler_->get_downstream_connection()); if(rv != 0) { // If downstream connection fails, issue RST_STREAM. rst_stream(downstream.get(), SPDYLAY_INTERNAL_ERROR); @@ -583,8 +580,7 @@ void spdy_downstream_readcb(bufferevent *bev, void *ptr) // on_stream_close_callback. upstream->rst_stream(downstream, infer_upstream_rst_stream_status_code (downstream->get_response_rst_stream_error_code())); - downstream->set_downstream_connection(nullptr); - delete dconn; + downstream->pop_downstream_connection(); dconn = nullptr; } else { auto rv = downstream->on_read(); @@ -604,8 +600,7 @@ void spdy_downstream_readcb(bufferevent *bev, void *ptr) downstream->set_response_state(Downstream::MSG_COMPLETE); // Clearly, we have to close downstream connection on http parser // failure. - downstream->set_downstream_connection(nullptr); - delete dconn; + downstream->pop_downstream_connection(); dconn = nullptr; } } @@ -667,8 +662,7 @@ void spdy_downstream_eventcb(bufferevent *bev, short events, void *ptr) // Delete downstream connection. If we don't delete it here, it // will be pooled in on_stream_close_callback. - downstream->set_downstream_connection(nullptr); - delete dconn; + downstream->pop_downstream_connection(); dconn = nullptr; // downstream wil be deleted in on_stream_close_callback. if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) { @@ -723,8 +717,7 @@ void spdy_downstream_eventcb(bufferevent *bev, short events, void *ptr) // Delete downstream connection. If we don't delete it here, it // will be pooled in on_stream_close_callback. - downstream->set_downstream_connection(nullptr); - delete dconn; + downstream->pop_downstream_connection(); dconn = nullptr; if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {