diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 1ae25da5816..aa43ceaf85b 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/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: diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index c5d1fe35b73..3a693ac2821 100644 --- a/mgmt/RecordsConfig.cc +++ b/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} diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc index 85c10b5c8c8..4327e71fb95 100644 --- a/proxy/http2/HTTP2.cc +++ b/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})); diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h index 8863ff4ee96..471b5452a39 100644 --- a/proxy/http2/HTTP2.h +++ b/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(); }; diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc index 277c1df2cfb..0bdccaae288 100644 --- a/proxy/http2/Http2ClientSession.cc +++ b/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; diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index 4191a90ffcb..05323708b63 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/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(); } } } diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h index 916c3ff9e34..7dc291d1838 100644 --- a/proxy/http2/Http2ConnectionState.h +++ b/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; };