From a8c389f3ac793086c88f58e43a157916d85a63e0 Mon Sep 17 00:00:00 2001 From: Masaori Koshiba Date: Wed, 15 Feb 2017 17:31:54 +0900 Subject: [PATCH] Set nullptr to ua_session after it is destoryed Issue: Crash by EXC_BAD_ACCESS in Http2ConnectionState::release_stream() under heavy load Cause: While total_connections_in is larger than max_connections_per_thread_in (in NetHandler::manage_keep_alive_queue()), Http2ConnectionState::release_stream() is called recurcively from add_to_keep_alive_queue(). At the bottom of recursion, ua_session is destroyed and Http2ConnectionState::release_stream() access to it. Fix: 1. Set nullptr to ua_session after it is destoryed 2. Swap calls of add_to_keep_alive_queue() and cancel_active_timeout() for ua_session nullptr check 3. Check m_active of ua_session to reduce recursion --- proxy/ProxyClientSession.h | 6 ++++++ proxy/http2/Http2ConnectionState.cc | 12 +++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/proxy/ProxyClientSession.h b/proxy/ProxyClientSession.h index 1539e935f7f..4bd9dc62bf3 100644 --- a/proxy/ProxyClientSession.h +++ b/proxy/ProxyClientSession.h @@ -107,6 +107,12 @@ class ProxyClientSession : public VConnection return this->api_hooks.has_hooks() || http_global_hooks->has_hooks(); } + bool + is_active() const + { + return m_active; + } + // Initiate an API hook invocation. void do_api_callout(TSHttpHookID id); diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index 69a4b3bab08..65a694ae9bf 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/proxy/http2/Http2ConnectionState.cc @@ -1055,18 +1055,20 @@ Http2ConnectionState::release_stream(Http2Stream *stream) stream_list.remove(stream); } - // If the number of clients is 0, then mark the connection as inactive - if (total_client_streams_count == 0 && ua_session) { + // If the number of clients is 0 and ua_session is active, then mark the connection as inactive + if (total_client_streams_count == 0 && ua_session && ua_session->is_active()) { ua_session->clear_session_active(); - if (ua_session->get_netvc()) { - ua_session->get_netvc()->add_to_keep_alive_queue(); - ua_session->get_netvc()->cancel_active_timeout(); + UnixNetVConnection *vc = static_cast(ua_session->get_netvc()); + if (vc) { + vc->cancel_active_timeout(); + vc->add_to_keep_alive_queue(); } } if (ua_session && fini_received && total_client_streams_count == 0) { // We were shutting down, go ahead and terminate the session ua_session->destroy(); + ua_session = nullptr; } }