From 6e20007706db8e0e861530abed426d55353fcf03 Mon Sep 17 00:00:00 2001 From: Masaori Koshiba Date: Mon, 9 Mar 2020 11:01:09 +0900 Subject: [PATCH] Cleanup: check activity of Http2Stream by ActivityCop --- iocore/net/NetTimeout.h | 263 +++++++++++++++++++++++++++++ proxy/http2/Http2ConnectionState.h | 10 ++ proxy/http2/Http2Stream.cc | 71 +++----- proxy/http2/Http2Stream.h | 17 +- 4 files changed, 300 insertions(+), 61 deletions(-) create mode 100644 iocore/net/NetTimeout.h diff --git a/iocore/net/NetTimeout.h b/iocore/net/NetTimeout.h new file mode 100644 index 00000000000..dcc584e0dfa --- /dev/null +++ b/iocore/net/NetTimeout.h @@ -0,0 +1,263 @@ +/** @file + + NetTimeout + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#pragma once + +#include "tscore/List.h" +#include "tscore/ink_hrtime.h" + +#include "I_EventSystem.h" + +/** + NetTimeout - handle active & inactive timeout + */ +class NetTimeout +{ +public: + void set_active_timeout(ink_hrtime timeout_in); + void set_inactive_timeout(ink_hrtime timeout_in); + void cancel_active_timeout(); + void cancel_inactive_timeout(); + void reset_active_timeout(); + void reset_inactive_timeout(); + bool is_active_timeout_expired(ink_hrtime now); + bool is_inactive_timeout_expired(ink_hrtime now); + ink_hrtime idle_time(ink_hrtime now); + void update_inactivity(); + +private: + ink_hrtime _active_timeout_in = 0; + ink_hrtime _inactive_timeout_in = 0; + ink_hrtime _next_active_timeout_at = 0; + ink_hrtime _next_inactive_timeout_at = 0; +}; + +/** + ActivityCop - Check activity of T in the List in every @f seconds + + T have to handle VC_EVENT_ACTIVE_TIMEOUT and VC_EVENT_INACTIVITY_TIMEOUT events. + + TODO: add concepts like below with C++20 + ``` + template > + concept Timeoutable = requires(T *t, List *list, ink_hrtime time) { + t->handleEvent(); + t->head; + {list->next(t)} -> std::convertible_to; + {t->is_active_timeout_expired(time)} -> std::same_as; + {t->is_inactive_timeout_expired(time)} -> std::same_as; + }; + ``` + */ +template > class ActivityCop : Continuation +{ +public: + ActivityCop(){}; + ActivityCop(Ptr &m, List *l, int f); + + void start(); + void stop(); + int check_activity(int event, Event *e); + +private: + Event *_event = nullptr; + List *_list = nullptr; + int _freq = 1; +}; + +//// +// Inline functions + +// +// NetTimeout +// +inline void +NetTimeout::set_active_timeout(ink_hrtime timeout_in) +{ + if (timeout_in == 0) { + return; + } + + _active_timeout_in = timeout_in; + _next_active_timeout_at = Thread::get_hrtime() + timeout_in; +} + +inline void +NetTimeout::set_inactive_timeout(ink_hrtime timeout_in) +{ + if (timeout_in == 0) { + return; + } + + _inactive_timeout_in = timeout_in; + _next_inactive_timeout_at = Thread::get_hrtime() + timeout_in; +} + +inline void +NetTimeout::cancel_active_timeout() +{ + _active_timeout_in = 0; + _next_active_timeout_at = 0; +} + +inline void +NetTimeout::cancel_inactive_timeout() +{ + _inactive_timeout_in = 0; + _next_inactive_timeout_at = 0; +} + +inline void +NetTimeout::reset_active_timeout() +{ + if (_active_timeout_in == 0) { + return; + } + + _next_active_timeout_at = Thread::get_hrtime() + _active_timeout_in; +} + +inline void +NetTimeout::reset_inactive_timeout() +{ + if (_inactive_timeout_in == 0) { + return; + } + + _next_inactive_timeout_at = Thread::get_hrtime() + _inactive_timeout_in; +} + +inline bool +NetTimeout::is_active_timeout_expired(ink_hrtime now) +{ + ink_assert(now > 0); + + if (_active_timeout_in == 0) { + return false; + } + + if (0 < _next_active_timeout_at && _next_active_timeout_at < now) { + Debug("activity_cop", "active timeout cont=%p now=%" PRId64 " timeout_at=%" PRId64 " timeout_in=%" PRId64, this, + ink_hrtime_to_sec(now), ink_hrtime_to_sec(_next_active_timeout_at), ink_hrtime_to_sec(_active_timeout_in)); + return true; + } + + return false; +} + +inline bool +NetTimeout::is_inactive_timeout_expired(ink_hrtime now) +{ + ink_assert(now > 0); + + if (_inactive_timeout_in == 0) { + return false; + } + + if (0 < _next_inactive_timeout_at && _next_inactive_timeout_at < now) { + Debug("activity_cop", "inactive timeout cont=%p now=%" PRId64 " timeout_at=%" PRId64 " timeout_in=%" PRId64, this, + ink_hrtime_to_sec(now), ink_hrtime_to_sec(_next_inactive_timeout_at), ink_hrtime_to_sec(_inactive_timeout_in)); + return true; + } + + return false; +} + +/** + Return how log this was inactive. + */ +inline ink_hrtime +NetTimeout::idle_time(ink_hrtime now) +{ + if (now < _next_inactive_timeout_at) { + return 0; + } + + return ink_hrtime_to_sec((now - _next_inactive_timeout_at) + _inactive_timeout_in); +} + +inline void +NetTimeout::update_inactivity() +{ + if (_inactive_timeout_in == 0) { + return; + } + + _next_inactive_timeout_at = Thread::get_hrtime() + _inactive_timeout_in; +} + +// +// ActivityCop +// +template +inline ActivityCop::ActivityCop(Ptr &m, List *l, int f) : Continuation(m.get()), _list(l), _freq(f) +{ + SET_HANDLER((&ActivityCop::check_activity)); +} + +template +inline void +ActivityCop::start() +{ + _event = this_ethread()->schedule_every(this, HRTIME_SECONDS(_freq)); +} + +template +inline void +ActivityCop::stop() +{ + _event->cancel(); +} + +template +inline int +ActivityCop::check_activity(int /* event */, Event *e) +{ + ink_hrtime now = Thread::get_hrtime(); + + // Traverse list & check inactivity or activity + T *t = _list->head; + while (t) { + T *next = _list->next(t); + if (t->mutex == nullptr) { + t = next; + continue; + } + + MUTEX_TRY_LOCK(lock, t->mutex, this_ethread()); + if (!lock.is_locked()) { + t = next; + continue; + } + + if (t->is_inactive_timeout_expired(now)) { + t->handleEvent(VC_EVENT_INACTIVITY_TIMEOUT, e); + } else if (t->is_active_timeout_expired(now)) { + t->handleEvent(VC_EVENT_ACTIVE_TIMEOUT, e); + } + + t = next; + } + + return EVENT_DONE; +} diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h index 8a83fa77ea4..7c81710e2af 100644 --- a/proxy/http2/Http2ConnectionState.h +++ b/proxy/http2/Http2ConnectionState.h @@ -24,6 +24,9 @@ #pragma once #include + +#include "NetTimeout.h" + #include "HTTP2.h" #include "HPACK.h" #include "Http2Stream.h" @@ -122,6 +125,7 @@ class Http2ConnectionState : public Continuation HpackHandle *local_hpack_handle = nullptr; HpackHandle *remote_hpack_handle = nullptr; DependencyTree *dependency_tree = nullptr; + ActivityCop _cop; // Settings. Http2ConnectionSettings server_settings; @@ -135,6 +139,9 @@ class Http2ConnectionState : public Continuation if (Http2::stream_priority_enabled) { dependency_tree = new DependencyTree(Http2::max_concurrent_streams_in); } + + _cop = ActivityCop(this->mutex, &stream_list, 1); + _cop.start(); } void @@ -145,6 +152,9 @@ class Http2ConnectionState : public Continuation return; } in_destroy = true; + + _cop.stop(); + if (shutdown_cont_event) { shutdown_cont_event->cancel(); shutdown_cont_event = nullptr; diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc index 83722b89256..7863cb2a8a7 100644 --- a/proxy/http2/Http2Stream.cc +++ b/proxy/http2/Http2Stream.cc @@ -21,8 +21,9 @@ limitations under the License. */ -#include "HTTP2.h" #include "Http2Stream.h" + +#include "HTTP2.h" #include "Http2ClientSession.h" #include "../http/HttpSM.h" @@ -81,14 +82,6 @@ Http2Stream::main_event_handler(int event, void *edata) return 0; } else if (e == cross_thread_event) { cross_thread_event = nullptr; - } else if (e == active_event) { - event = VC_EVENT_ACTIVE_TIMEOUT; - active_event = nullptr; - } else if (e == inactive_event) { - if (inactive_timeout_at && inactive_timeout_at < Thread::get_hrtime()) { - event = VC_EVENT_INACTIVITY_TIMEOUT; - clear_inactive_timer(); - } } else if (e == read_event) { read_event = nullptr; } else if (e == write_event) { @@ -108,7 +101,7 @@ Http2Stream::main_event_handler(int event, void *edata) break; case VC_EVENT_WRITE_READY: case VC_EVENT_WRITE_COMPLETE: - inactive_timeout_at = Thread::get_hrtime() + inactive_timeout; + _timeout.update_inactivity(); if (e->cookie == &write_vio) { if (write_vio.mutex && write_vio.cont && this->_sm) { this->signal_write_event(event); @@ -119,7 +112,7 @@ Http2Stream::main_event_handler(int event, void *edata) break; case VC_EVENT_READ_COMPLETE: case VC_EVENT_READ_READY: - inactive_timeout_at = Thread::get_hrtime() + inactive_timeout; + _timeout.update_inactivity(); if (e->cookie == &read_vio) { if (read_vio.mutex && read_vio.cont && this->_sm) { signal_read_event(event); @@ -363,7 +356,6 @@ Http2Stream::do_io_close(int /* flags */) h2_proxy_ssn->connection_state.send_data_frames(this); } - clear_timers(); clear_io_events(); // Wait until transaction_done is called from HttpSM to signal that the TXN_CLOSE hook has been executed @@ -429,7 +421,6 @@ Http2Stream::initiating_close() // TXN_CLOSE has been sent // _proxy_ssn = NULL; - clear_timers(); clear_io_events(); // This should result in do_io_close or release being called. That will schedule the final @@ -514,7 +505,7 @@ Http2Stream::update_read_request(int64_t read_len, bool call_update, bool check_ int64_t read_avail = this->read_vio.buffer.writer()->max_read_avail(); if (read_avail > 0 || send_event == VC_EVENT_READ_COMPLETE) { if (call_update) { // Safe to call vio handler directly - inactive_timeout_at = Thread::get_hrtime() + inactive_timeout; + _timeout.update_inactivity(); if (read_vio.cont && this->_sm) { read_vio.cont->handleEvent(send_event, &read_vio); } @@ -639,7 +630,7 @@ Http2Stream::signal_read_event(int event) MUTEX_TRY_LOCK(lock, read_vio.cont->mutex, this_ethread()); if (lock.is_locked()) { - inactive_timeout_at = Thread::get_hrtime() + inactive_timeout; + _timeout.update_inactivity(); this->read_vio.cont->handleEvent(event, &this->read_vio); } else { if (this->_read_vio_event) { @@ -658,7 +649,7 @@ Http2Stream::signal_write_event(int event) MUTEX_TRY_LOCK(lock, write_vio.cont->mutex, this_ethread()); if (lock.is_locked()) { - inactive_timeout_at = Thread::get_hrtime() + inactive_timeout; + _timeout.update_inactivity(); this->write_vio.cont->handleEvent(event, &this->write_vio); } else { if (this->_write_vio_event) { @@ -704,7 +695,7 @@ void Http2Stream::send_response_body(bool call_update) { Http2ClientSession *h2_proxy_ssn = static_cast(this->_proxy_ssn); - inactive_timeout_at = Thread::get_hrtime() + inactive_timeout; + _timeout.update_inactivity(); if (Http2::stream_priority_enabled) { SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread()); @@ -805,7 +796,6 @@ Http2Stream::destroy() if (header_blocks) { ats_free(header_blocks); } - clear_timers(); clear_io_events(); http_parser_clear(&http_parser); @@ -822,56 +812,37 @@ Http2Stream::response_get_data_reader() const void Http2Stream::set_active_timeout(ink_hrtime timeout_in) { - active_timeout = timeout_in; - clear_active_timer(); - if (active_timeout > 0) { - active_event = this_ethread()->schedule_in(this, active_timeout); - } + _timeout.set_active_timeout(timeout_in); } void Http2Stream::set_inactivity_timeout(ink_hrtime timeout_in) { - inactive_timeout = timeout_in; - if (inactive_timeout > 0) { - inactive_timeout_at = Thread::get_hrtime() + inactive_timeout; - if (!inactive_event) { - inactive_event = this_ethread()->schedule_every(this, HRTIME_SECONDS(1)); - } - } else { - clear_inactive_timer(); - } + _timeout.set_inactive_timeout(timeout_in); } void -Http2Stream::cancel_inactivity_timeout() +Http2Stream::cancel_active_timeout() { - set_inactivity_timeout(0); + _timeout.cancel_active_timeout(); } + void -Http2Stream::clear_inactive_timer() +Http2Stream::cancel_inactivity_timeout() { - inactive_timeout_at = 0; - if (inactive_event) { - inactive_event->cancel(); - inactive_event = nullptr; - } + _timeout.cancel_inactive_timeout(); } -void -Http2Stream::clear_active_timer() +bool +Http2Stream::is_active_timeout_expired(ink_hrtime now) { - if (active_event) { - active_event->cancel(); - active_event = nullptr; - } + return _timeout.is_active_timeout_expired(now); } -void -Http2Stream::clear_timers() +bool +Http2Stream::is_inactive_timeout_expired(ink_hrtime now) { - clear_inactive_timer(); - clear_active_timer(); + return _timeout.is_inactive_timeout_expired(now); } void diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h index df560a1c386..1b184c1249d 100644 --- a/proxy/http2/Http2Stream.h +++ b/proxy/http2/Http2Stream.h @@ -23,6 +23,8 @@ #pragma once +#include "NetTimeout.h" + #include "HTTP2.h" #include "ProxyTransaction.h" #include "Http2DebugNames.h" @@ -94,7 +96,10 @@ class Http2Stream : public ProxyTransaction // Accessors void set_active_timeout(ink_hrtime timeout_in) override; void set_inactivity_timeout(ink_hrtime timeout_in) override; + void cancel_active_timeout(); void cancel_inactivity_timeout() override; + bool is_active_timeout_expired(ink_hrtime now); + bool is_inactive_timeout_expired(ink_hrtime now); bool allow_half_open() const override; bool is_first_transaction() const override; @@ -104,9 +109,6 @@ class Http2Stream : public ProxyTransaction int get_transaction_priority_weight() const override; int get_transaction_priority_dependence() const override; - void clear_inactive_timer(); - void clear_active_timer(); - void clear_timers(); void clear_io_events(); bool is_client_state_writeable() const; @@ -158,6 +160,7 @@ class Http2Stream : public ProxyTransaction */ bool _switch_thread_if_not_on_right_thread(int event, void *edata); + NetTimeout _timeout{}; HTTPParser http_parser; EThread *_thread = nullptr; Http2StreamId _id; @@ -210,14 +213,6 @@ class Http2Stream : public ProxyTransaction Event *cross_thread_event = nullptr; Event *buffer_full_write_event = nullptr; - // Support stream-specific timeouts - ink_hrtime active_timeout = 0; - Event *active_event = nullptr; - - ink_hrtime inactive_timeout = 0; - ink_hrtime inactive_timeout_at = 0; - Event *inactive_event = nullptr; - Event *read_event = nullptr; Event *write_event = nullptr; Event *_read_vio_event = nullptr;