nghttpx: Use std::unique_ptr for DownstreamConnection

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<DownstreamConnection> 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<DownstreamConnection>
+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<Http2DownstreamConnection>(this);
     } else {
-      return new HttpDownstreamConnection(this);
+      return util::make_unique<HttpDownstreamConnection>(this);
     }
-  } else {
+  }
-    auto dconn = *std::begin(dconn_pool_);
+
-    dconn_pool_.erase(dconn);
+  auto dconn = std::unique_ptr<DownstreamConnection>(*std::begin(dconn_pool_));
+  dconn_pool_.erase(dconn.get());
   if(LOG_ENABLED(INFO)) {
-      CLOG(INFO, this) << "Reuse downstream connection DCONN:" << dconn
+    CLOG(INFO, this) << "Reuse downstream connection DCONN:" << dconn.get()
                      << " from pool";
   }
+
   return dconn;
-  }
 }
 
 size_t ClientHandler::get_outbuf_length()

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<DownstreamConnection> dconn);
   void remove_downstream_connection(DownstreamConnection *dconn);
-  DownstreamConnection* get_downstream_connection();
+  std::unique_ptr<DownstreamConnection> get_downstream_connection();
   size_t get_outbuf_length();
   SSL* get_ssl() const;
   void set_http2_session(Http2Session *http2session);

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<DownstreamConnection> 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<DownstreamConnection>(dconn_.release()));
+}
+
+void Downstream::release_downstream_connection()
+{
+  dconn_.release();
 }
 
 DownstreamConnection* Downstream::get_downstream_connection()
 {
-  return dconn_;
+  return dconn_.get();
+}
+
+std::unique_ptr<DownstreamConnection> Downstream::pop_downstream_connection()
+{
+  return std::unique_ptr<DownstreamConnection>(dconn_.release());
 }
 
 void Downstream::pause_read(IOCtrlReason reason)

src/shrpx_downstream.h

@@ -31,6 +31,7 @@
 
 #include <vector>
 #include <string>
+#include <memory>
 
 #include <event.h>
 #include <event2/bufferevent.h>
@@ -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<DownstreamConnection> 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<DownstreamConnection> 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<DownstreamConnection> dconn_;
   // This buffer is used to temporarily store downstream response
   // body. nghttp2 library reads data from this in the callback.
   evbuffer *response_body_buf_;

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,

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();
+      downstream->detach_downstream_connection();
-
-      if(dconn) {
-        dconn->detach_downstream(downstream);
-      }
     }
 
     upstream->remove_downstream(downstream);
@@ -399,8 +395,8 @@ void Http2Upstream::initiate_downstream(std::unique_ptr<Downstream> downstream)
 {
   int rv;
 
-  auto dconn = handler_->get_downstream_connection();
+  rv = downstream->attach_downstream_connection
-  rv = dconn->attach_downstream(downstream.get());
+    (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);
+    downstream->pop_downstream_connection();
-    delete dconn;
+    // 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);
+      downstream->pop_downstream_connection();
-      delete dconn;
+      // 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);
+    downstream->pop_downstream_connection();
-    delete dconn;
+    // 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);
+    downstream->pop_downstream_connection();
-    delete dconn;
+    // dconn was deleted
     dconn = nullptr;
 
     if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {

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_ = nullptr;
-  downstream_ = 0;
   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()

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 =  downstream->attach_downstream_connection
-
+    (upstream->get_client_handler()->get_downstream_connection());
-  rv =  dconn->attach_downstream(downstream);
 
   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);
+        downstream->pop_downstream_connection();
-
+        // dconn was deleted
-        delete dconn;
       } 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);
+      downstream->pop_downstream_connection();
-
-      delete dconn;
 
       dconn = nullptr;
     } else {
       // Keep-alive
-      dconn->detach_downstream(downstream);
+      downstream->detach_downstream_connection();
     }
   }
 

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();
+      downstream->detach_downstream_connection();
-      if(dconn) {
-        dconn->detach_downstream(downstream);
-      }
     }
     upstream->remove_downstream(downstream);
     // downstrea was deleted
@@ -265,8 +262,8 @@ void SpdyUpstream::maintain_downstream_concurrency()
 
 void SpdyUpstream::initiate_downstream(std::unique_ptr<Downstream> downstream)
 {
-  auto dconn = handler_->get_downstream_connection();
+  int rv = downstream->attach_downstream_connection
-  int rv = dconn->attach_downstream(downstream.get());
+    (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);
+    downstream->pop_downstream_connection();
-    delete dconn;
     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);
+      downstream->pop_downstream_connection();
-      delete dconn;
       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);
+    downstream->pop_downstream_connection();
-    delete dconn;
     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);
+    downstream->pop_downstream_connection();
-    delete dconn;
     dconn = nullptr;
 
     if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {