You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2018/05/25 13:35:29 UTC
[trafficserver] branch master updated: Add zombie-event option to
debug leaked Http2ClientSessions.
This is an automated email from the ASF dual-hosted git repository.
shinrich pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new aa5ead6 Add zombie-event option to debug leaked Http2ClientSessions.
aa5ead6 is described below
commit aa5ead6edc46e46baa3832f512951ebe9ca83463
Author: Susan Hinrichs <sh...@apache.org>
AuthorDate: Tue May 22 11:18:34 2018 -0500
Add zombie-event option to debug leaked Http2ClientSessions.
---
doc/admin-guide/files/records.config.en.rst | 9 ++++++++
mgmt/RecordsConfig.cc | 2 ++
proxy/http2/HTTP2.cc | 2 ++
proxy/http2/HTTP2.h | 1 +
proxy/http2/Http2ClientSession.cc | 10 +++++++--
proxy/http2/Http2ConnectionState.cc | 33 +++++++++++++++++++++++++++--
proxy/http2/Http2ConnectionState.h | 18 ++++++++++++++++
7 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index 1ae25da..aa43cea 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 0d459d1..e2c7513 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1298,6 +1298,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 85c10b5..4327e71 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 8863ff4..471b545 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -377,6 +377,7 @@ public:
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 277c1df..0bdccaa 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 4191a90..0532370 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 916c3ff..7dc291d 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -275,6 +275,23 @@ public:
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 @@ private:
Http2ShutdownState shutdown_state = HTTP2_SHUTDOWN_NONE;
Event *shutdown_cont_event = nullptr;
Event *fini_event = nullptr;
+ Event *zombie_event = nullptr;
};
--
To stop receiving notification emails like this one, please contact
shinrich@apache.org.