From cb7269f3345c1965597cc1ebdf2d053ec3894a9b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 4 Jun 2016 12:16:31 +0900 Subject: [PATCH] nghttpx: Close and disallow h1 backend connection on backend replacement --- src/shrpx_downstream_connection_pool.cc | 6 +++++- src/shrpx_downstream_connection_pool.h | 1 + src/shrpx_http_downstream_connection.cc | 8 +++++++- src/shrpx_http_downstream_connection.h | 2 +- src/shrpx_worker.cc | 12 +++++++----- src/shrpx_worker.h | 4 ++++ 6 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/shrpx_downstream_connection_pool.cc b/src/shrpx_downstream_connection_pool.cc index c58ed81a..0ee66b60 100644 --- a/src/shrpx_downstream_connection_pool.cc +++ b/src/shrpx_downstream_connection_pool.cc @@ -29,10 +29,14 @@ namespace shrpx { DownstreamConnectionPool::DownstreamConnectionPool() {} -DownstreamConnectionPool::~DownstreamConnectionPool() { +DownstreamConnectionPool::~DownstreamConnectionPool() { remove_all(); } + +void DownstreamConnectionPool::remove_all() { for (auto dconn : pool_) { delete dconn; } + + pool_.clear(); } void DownstreamConnectionPool::add_downstream_connection( diff --git a/src/shrpx_downstream_connection_pool.h b/src/shrpx_downstream_connection_pool.h index c2edce45..34dc30d8 100644 --- a/src/shrpx_downstream_connection_pool.h +++ b/src/shrpx_downstream_connection_pool.h @@ -42,6 +42,7 @@ public: void add_downstream_connection(std::unique_ptr dconn); std::unique_ptr pop_downstream_connection(); void remove_downstream_connection(DownstreamConnection *dconn); + void remove_all(); private: std::set pool_; diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 0bd1aeac..14d1e000 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -166,7 +166,11 @@ HttpDownstreamConnection::HttpDownstreamConnection( ioctrl_(&conn_.rlimit), response_htp_{0} {} -HttpDownstreamConnection::~HttpDownstreamConnection() {} +HttpDownstreamConnection::~HttpDownstreamConnection() { + if (LOG_ENABLED(INFO)) { + DCLOG(INFO, this) << "Deleted"; + } +} int HttpDownstreamConnection::attach_downstream(Downstream *downstream) { if (LOG_ENABLED(INFO)) { @@ -1173,4 +1177,6 @@ HttpDownstreamConnection::get_downstream_addr_group() const { DownstreamAddr *HttpDownstreamConnection::get_addr() const { return addr_; } +bool HttpDownstreamConnection::poolable() const { return !group_->retired; } + } // namespace shrpx diff --git a/src/shrpx_http_downstream_connection.h b/src/shrpx_http_downstream_connection.h index cebed9f0..40b53f44 100644 --- a/src/shrpx_http_downstream_connection.h +++ b/src/shrpx_http_downstream_connection.h @@ -61,7 +61,7 @@ public: virtual void on_upstream_change(Upstream *upstream); - virtual bool poolable() const { return true; } + virtual bool poolable() const; virtual DownstreamAddrGroup *get_downstream_addr_group() const; diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index 64e9d0e8..1a7cc90e 100644 --- a/src/shrpx_worker.cc +++ b/src/shrpx_worker.cc @@ -141,6 +141,11 @@ Worker::Worker(struct ev_loop *loop, SSL_CTX *sv_ssl_ctx, SSL_CTX *cl_ssl_ctx, void Worker::replace_downstream_config( std::shared_ptr downstreamconf) { + for (auto &g : downstream_addr_groups_) { + g->retired = true; + g->shared_addr->dconn_pool.remove_all(); + } + downstreamconf_ = downstreamconf; downstream_addr_groups_ = std::vector>( @@ -153,14 +158,11 @@ void Worker::replace_downstream_config( dst = std::make_shared(); dst->pattern = src.pattern; + // TODO for some reason, clang-3.6 which comes with Ubuntu 15.10 + // does not value initialize with std::make_shared. auto shared_addr = std::make_shared(); - // TODO for some reason, clang-3.6 which comes with Ubuntu 15.10 - // does not value initialize SharedDownstreamAddr above. - shared_addr->next = 0; shared_addr->addrs.resize(src.addrs.size()); - shared_addr->http1_pri = {}; - shared_addr->http2_pri = {}; size_t num_http1 = 0; size_t num_http2 = 0; diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h index c3e3e04a..1a334723 100644 --- a/src/shrpx_worker.h +++ b/src/shrpx_worker.h @@ -144,6 +144,10 @@ struct SharedDownstreamAddr { struct DownstreamAddrGroup { ImmutableString pattern; std::shared_ptr shared_addr; + // true if this group is no longer used for new request. If this is + // true, the connection made using one of address in shared_addr + // must not be pooled. + bool retired; }; struct WorkerStat {