nghttpx: Fix bugs and crash when affinity is enabled

src/shrpx_api_downstream_connection.cc

@@ -302,4 +302,6 @@ APIDownstreamConnection::get_downstream_addr_group() const {
   return nullptr;
 }
 
+DownstreamAddr *APIDownstreamConnection::get_addr() const { return nullptr; }
+
 } // namespace shrpx

src/shrpx_api_downstream_connection.h

@@ -55,6 +55,7 @@ public:
   virtual bool poolable() const;
 
   virtual DownstreamAddrGroup *get_downstream_addr_group() const;
+  virtual DownstreamAddr *get_addr() const;
 
   int send_reply(unsigned int http_status, int api_status);
 

src/shrpx_client_handler.cc

@@ -669,8 +669,18 @@ void ClientHandler::pool_downstream_connection(
                      << " in group " << group;
   }
 
+  auto &shared_addr = group->shared_addr;
+
+  if (shared_addr->affinity == AFFINITY_NONE) {
     auto &dconn_pool = group->shared_addr->dconn_pool;
     dconn_pool.add_downstream_connection(std::move(dconn));
+
+    return;
+  }
+
+  auto addr = dconn->get_addr();
+  auto &dconn_pool = addr->dconn_pool;
+  dconn_pool->add_downstream_connection(std::move(dconn));
 }
 
 void ClientHandler::remove_downstream_connection(DownstreamConnection *dconn) {
@@ -715,7 +725,6 @@ Http2Session *ClientHandler::select_http2_session_with_affinity(
 
   if (addr->http2_extra_freelist.size()) {
     auto session = addr->http2_extra_freelist.head;
-    session->remove_from_freelist();
 
     if (LOG_ENABLED(INFO)) {
       CLOG(INFO, this) << "Use Http2Session " << session
@@ -727,8 +736,8 @@ Http2Session *ClientHandler::select_http2_session_with_affinity(
         CLOG(INFO, this) << "Maximum streams are reached for Http2Session("
                          << session << ").";
       }
-    } else {
+
-      session->add_to_avail_freelist();
+      session->remove_from_freelist();
     }
     return session;
   }
@@ -740,7 +749,7 @@ Http2Session *ClientHandler::select_http2_session_with_affinity(
     CLOG(INFO, this) << "Create new Http2Session " << session;
   }
 
-  session->add_to_avail_freelist();
+  session->add_to_extra_freelist();
 
   return session;
 }

src/shrpx_downstream_connection.h

@@ -35,6 +35,7 @@ class ClientHandler;
 class Upstream;
 class Downstream;
 struct DownstreamAddrGroup;
+struct DownstreamAddr;
 
 class DownstreamConnection {
 public:
@@ -61,6 +62,7 @@ public:
   virtual bool poolable() const = 0;
 
   virtual DownstreamAddrGroup *get_downstream_addr_group() const = 0;
+  virtual DownstreamAddr *get_addr() const = 0;
 
   void set_client_handler(ClientHandler *client_handler);
   ClientHandler *get_client_handler();

src/shrpx_http2_downstream_connection.cc

@@ -543,4 +543,6 @@ Http2DownstreamConnection::get_downstream_addr_group() const {
   return http2session_->get_downstream_addr_group();
 }
 
+DownstreamAddr *Http2DownstreamConnection::get_addr() const { return nullptr; }
+
 } // namespace shrpx

src/shrpx_http2_downstream_connection.h

@@ -65,6 +65,7 @@ public:
   virtual bool poolable() const { return false; }
 
   virtual DownstreamAddrGroup *get_downstream_addr_group() const;
+  virtual DownstreamAddr *get_addr() const;
 
   int send();
 

src/shrpx_http_downstream_connection.cc

@@ -562,6 +562,24 @@ int HttpDownstreamConnection::end_upload_data() {
   return 0;
 }
 
+namespace {
+void remove_from_pool(HttpDownstreamConnection *dconn) {
+  auto group = dconn->get_downstream_addr_group();
+  auto &shared_addr = group->shared_addr;
+
+  if (shared_addr->affinity == AFFINITY_NONE) {
+    auto &dconn_pool =
+        dconn->get_downstream_addr_group()->shared_addr->dconn_pool;
+    dconn_pool.remove_downstream_connection(dconn);
+    return;
+  }
+
+  auto addr = dconn->get_addr();
+  auto &dconn_pool = addr->dconn_pool;
+  dconn_pool->remove_downstream_connection(dconn);
+}
+} // namespace
+
 namespace {
 void idle_readcb(struct ev_loop *loop, ev_io *w, int revents) {
   auto conn = static_cast<Connection *>(w->data);
@@ -569,9 +587,8 @@ void idle_readcb(struct ev_loop *loop, ev_io *w, int revents) {
   if (LOG_ENABLED(INFO)) {
     DCLOG(INFO, dconn) << "Idle connection EOF";
   }
-  auto &dconn_pool =
+
-      dconn->get_downstream_addr_group()->shared_addr->dconn_pool;
+  remove_from_pool(dconn);
-  dconn_pool.remove_downstream_connection(dconn);
   // dconn was deleted
 }
 } // namespace
@@ -583,9 +600,8 @@ void idle_timeoutcb(struct ev_loop *loop, ev_timer *w, int revents) {
   if (LOG_ENABLED(INFO)) {
     DCLOG(INFO, dconn) << "Idle connection timeout";
   }
-  auto &dconn_pool =
+
-      dconn->get_downstream_addr_group()->shared_addr->dconn_pool;
+  remove_from_pool(dconn);
-  dconn_pool.remove_downstream_connection(dconn);
   // dconn was deleted
 }
 } // namespace

src/shrpx_http_downstream_connection.h

@@ -65,6 +65,7 @@ public:
   virtual bool poolable() const;
 
   virtual DownstreamAddrGroup *get_downstream_addr_group() const;
+  virtual DownstreamAddr *get_addr() const;
 
   int read_clear();
   int write_clear();
@@ -80,8 +81,6 @@ public:
 
   int noop();
 
-  DownstreamAddr *get_addr() const;
-
 private:
   Connection conn_;
   std::function<int(HttpDownstreamConnection &)> do_read_, do_write_,

src/shrpx_worker.h

@@ -96,6 +96,9 @@ struct DownstreamAddr {
   // Http2Session object created for this address.  This list chains
   // all Http2Session objects that is not in group scope
   // http2_avail_freelist, and is not reached in maximum concurrency.
+  //
+  // If session affinity is enabled, http2_avail_freelist is not used,
+  // and this list is solely used.
   DList<Http2Session> http2_extra_freelist;
   // true if Http2Session for this address is in group scope
   // SharedDownstreamAddr.http2_avail_freelist
@@ -129,6 +132,9 @@ struct SharedDownstreamAddr {
   // coalesce as much stream as possible in one Http2Session to fully
   // utilize TCP connection.
   //
+  // If session affinity is enabled, this list is not used.  Per
+  // address http2_extra_freelist is used instead.
+  //
   // TODO Verify that this approach performs better in performance
   // wise.
   DList<Http2Session> http2_avail_freelist;