Add zombie-event option to debug leaked Http2ClientSessions.

doc/admin-guide/files/records.config.en.rst

@@ -3518,6 +3518,15 @@ HTTP/2 Configuration
    misconfigured or misbehaving clients are opening a large number of
    connections without submitting requests.
 
+.. ts:cv:: CONFIG proxy.config.http2.zombie_debug_timeout_in INT 0
+   :reloadable:
+
+   This timeout enables the zombie debugging feature.  If it is non-zero, it sets a zombie event to go off that
+   many seconds in the future when the HTTP2 session reaches one but not both of the terminating events, i.e received 
+   a close event (via client goaway or timeout) and the number of active streams has gone to zero.  If the event is executed,
+   the Traffic Server process will assert.  This mechanism is useful to debug potential leaks in the HTTP2 Stream and Session 
+   processing.
+
 .. ts:cv:: CONFIG proxy.config.http2.push_diary_size INT 256
    :reloadable:
 

mgmt/RecordsConfig.cc

@@ -1300,6 +1300,8 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.http2.push_diary_size", RECD_INT, "256", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
   ,
+  {RECT_CONFIG, "proxy.config.http2.zombie_debug_timeout_in", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
+  ,
 
   //# Add LOCAL Records Here
   {RECT_LOCAL, "proxy.local.incoming_ip_to_bind", RECD_STRING, nullptr, RECU_NULL, RR_NULL, RECC_NULL, nullptr, RECA_NULL}

proxy/http2/HTTP2.cc

@@ -730,6 +730,7 @@ uint32_t Http2::accept_no_activity_timeout = 120;
 uint32_t Http2::no_activity_timeout_in     = 120;
 uint32_t Http2::active_timeout_in          = 0;
 uint32_t Http2::push_diary_size            = 256;
+uint32_t Http2::zombie_timeout_in          = 0;
 
 void
 Http2::init()
@@ -747,6 +748,7 @@ Http2::init()
   REC_EstablishStaticConfigInt32U(no_activity_timeout_in, "proxy.config.http2.no_activity_timeout_in");
   REC_EstablishStaticConfigInt32U(active_timeout_in, "proxy.config.http2.active_timeout_in");
   REC_EstablishStaticConfigInt32U(push_diary_size, "proxy.config.http2.push_diary_size");
+  REC_EstablishStaticConfigInt32U(zombie_timeout_in, "proxy.config.http2.zombie_debug_timeout_in");
 
   // If any settings is broken, ATS should not start
   ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, max_concurrent_streams_in}));

proxy/http2/HTTP2.h

@@ -377,6 +377,7 @@ class Http2
   static uint32_t no_activity_timeout_in;
   static uint32_t active_timeout_in;
   static uint32_t push_diary_size;
+  static uint32_t zombie_timeout_in;
 
   static void init();
 };

proxy/http2/Http2ClientSession.cc

@@ -307,9 +307,14 @@ Http2ClientSession::main_event_handler(int event, void *edata)
 
   switch (event) {
   case VC_EVENT_READ_COMPLETE:
-  case VC_EVENT_READ_READY:
-    retval = (this->*session_handler)(event, edata);
+  case VC_EVENT_READ_READY: {
+    bool is_zombie = connection_state.get_zombie_event() != nullptr;
+    retval         = (this->*session_handler)(event, edata);
+    if (is_zombie && connection_state.get_zombie_event() != nullptr) {
+      Warning("Processed read event for zombie session %" PRId64, connection_id());
+    }
     break;
+  }
 
   case HTTP2_SESSION_EVENT_XMIT: {
     Http2Frame *frame = (Http2Frame *)edata;
@@ -325,6 +330,7 @@ Http2ClientSession::main_event_handler(int event, void *edata)
   case VC_EVENT_INACTIVITY_TIMEOUT:
   case VC_EVENT_ERROR:
   case VC_EVENT_EOS:
+    this->set_dying_event(event);
     this->do_io_close();
     retval = 0;
     break;

proxy/http2/Http2ConnectionState.cc

@@ -73,6 +73,10 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
 
   Http2StreamDebug(cstate.ua_session, id, "Received DATA frame");
 
+  if (cstate.get_zombie_event()) {
+    Warning("Data frame for zombied session %" PRId64, cstate.ua_session->connection_id());
+  }
+
   // If a DATA frame is received whose stream identifier field is 0x0, the
   // recipient MUST
   // respond with a connection error of type PROTOCOL_ERROR.
@@ -363,6 +367,10 @@ rcv_priority_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
 
   Http2StreamDebug(cstate.ua_session, stream_id, "Received PRIORITY frame");
 
+  if (cstate.get_zombie_event()) {
+    Warning("Priority frame for zombied sessoin %" PRId64, cstate.ua_session->connection_id());
+  }
+
   // If a PRIORITY frame is received with a stream identifier of 0x0, the
   // recipient MUST respond with a connection error of type PROTOCOL_ERROR.
   if (stream_id == 0) {
@@ -488,6 +496,10 @@ rcv_settings_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
 
   Http2StreamDebug(cstate.ua_session, stream_id, "Received SETTINGS frame");
 
+  if (cstate.get_zombie_event()) {
+    Warning("Setting frame for zombied sessoin %" PRId64, cstate.ua_session->connection_id());
+  }
+
   // [RFC 7540] 6.5. The stream identifier for a SETTINGS frame MUST be zero.
   // If an endpoint receives a SETTINGS frame whose stream identifier field is
   // anything other than 0x0, the endpoint MUST respond with a connection
@@ -575,6 +587,8 @@ rcv_ping_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
 
   Http2StreamDebug(cstate.ua_session, stream_id, "Received PING frame");
 
+  cstate.schedule_zombie_event();
+
   //  If a PING frame is received with a stream identifier field value other
   //  than 0x0, the recipient MUST respond with a connection error of type
   //  PROTOCOL_ERROR.
@@ -840,7 +854,10 @@ static const http2_frame_dispatch frame_handlers[HTTP2_FRAME_TYPE_MAX] = {
 int
 Http2ConnectionState::main_event_handler(int event, void *edata)
 {
-  if (edata == fini_event) {
+  if (edata == zombie_event) {
+    // zombie session is still around. Assert
+    ink_release_assert(zombie_event == nullptr);
+  } else if (edata == fini_event) {
     fini_event = nullptr;
   }
   ++recursion;
@@ -984,7 +1001,10 @@ Http2ConnectionState::main_event_handler(int event, void *edata)
 int
 Http2ConnectionState::state_closed(int /* event */, void *edata)
 {
-  if (edata == fini_event) {
+  if (edata == zombie_event) {
+    // Zombie session is still around.  Assert!
+    ink_release_assert(zombie_event == nullptr);
+  } else if (edata == fini_event) {
     fini_event = nullptr;
   }
   return 0;
@@ -1057,6 +1077,11 @@ Http2ConnectionState::create_stream(Http2StreamId new_id, Http2Error &error)
   }
   ++total_client_streams_count;
 
+  if (zombie_event != nullptr) {
+    zombie_event->cancel();
+    zombie_event = nullptr;
+  }
+
   new_stream->set_parent(ua_session);
   new_stream->mutex                     = new_ProxyMutex();
   new_stream->is_first_transaction_flag = get_stream_requests() == 0;
@@ -1222,7 +1247,11 @@ Http2ConnectionState::release_stream(Http2Stream *stream)
         // ua_session = nullptr;
       } else if (shutdown_state == HTTP2_SHUTDOWN_IN_PROGRESS && fini_event == nullptr) {
         fini_event = this_ethread()->schedule_imm_local((Continuation *)this, HTTP2_SESSION_EVENT_FINI);
+      } else {
+        schedule_zombie_event();
       }
+    } else if (fini_received) {
+      schedule_zombie_event();
     }
   }
 }

proxy/http2/Http2ConnectionState.h

@@ -275,6 +275,23 @@ class Http2ConnectionState : public Continuation
   Http2ConnectionState(const Http2ConnectionState &) = delete;
   Http2ConnectionState &operator=(const Http2ConnectionState &) = delete;
 
+  Event *
+  get_zombie_event()
+  {
+    return zombie_event;
+  }
+
+  void
+  schedule_zombie_event()
+  {
+    if (Http2::zombie_timeout_in) { // If we have zombie debugging enabled
+      if (zombie_event) {
+        zombie_event->cancel();
+      }
+      zombie_event = this_ethread()->schedule_in(this, HRTIME_SECONDS(Http2::zombie_timeout_in));
+    }
+  }
+
 private:
   unsigned _adjust_concurrent_stream();
 
@@ -313,4 +330,5 @@ class Http2ConnectionState : public Continuation
   Http2ShutdownState shutdown_state = HTTP2_SHUTDOWN_NONE;
   Event *shutdown_cont_event        = nullptr;
   Event *fini_event                 = nullptr;
+  Event *zombie_event               = nullptr;
 };