This is an automated email from the ASF dual-hosted git repository. zwoop pushed a commit to branch 9.2.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 353cedbbd29d9da0c063d1b9794ee6e37d66a432 Author: Alan M. Carroll <[email protected]> AuthorDate: Wed Oct 20 11:18:29 2021 -0500 Promote class PendingAction from HttpSM.h for use in other classes. (#8423) (cherry picked from commit c72d43361b8d63cc8e7a190b5e91f55354afafd8) --- include/tscore/PendingAction.h | 129 +++++++++++++++++++++++++++++++++++++++++ proxy/http/HttpSM.cc | 38 ++++++------ proxy/http/HttpSM.h | 49 +--------------- 3 files changed, 149 insertions(+), 67 deletions(-) diff --git a/include/tscore/PendingAction.h b/include/tscore/PendingAction.h new file mode 100644 index 000000000..c23bb9cb6 --- /dev/null +++ b/include/tscore/PendingAction.h @@ -0,0 +1,129 @@ +/** @file + +Container for a pending @c Action. + +@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. +*/ + +/** Hold a pending @c Action. + * + * This is modeled on smart pointer classes. This class wraps a pointer to + * an @c Action (which is also the super type for @c Event). If cleared or + * re-assigned the current @c Action, if any, is canceled before the pointer is + * lost to avoid ghost actions triggering after the main continuation is gone. + * + * The class is aware of the special value @c ACTION_RESULT_DONE. If that is assigned + * the pending action will not be canceled or cleared. + */ +class PendingAction +{ + using self_type = PendingAction; + +public: + PendingAction() = default; + PendingAction(self_type const &) = delete; + ~PendingAction(); + self_type &operator=(self_type const &) = delete; + + /// Check if there is an action. + /// @return @c true if no action is present, @c false otherwise. + bool empty() const; + + /** Assign a new @a action. + * + * @param action The instance to store. + * @return @a this + * + * Any existing @c Action is canceled. + * Assignment is thread + */ + self_type &operator=(Action *action); + + /** Get the @c Continuation for the @c Action. + * + * @return A pointer to the continuation if there is an @c Action, @c nullptr if not. + */ + Continuation *get_continuation() const; + + /** Get the @c Action. + * + * @return A pointer to the @c Action is present, @c nullptr if not. + */ + Action *get() const; + + /** Clear the current @c Action if it is @a action. + * + * @param action @c Action to check. + * + * This clears the internal pointer without any side effect. it is used when the @c Action + * is handled and therefore should no longer be canceled. + */ + void clear_if_action_is(Action *action); + +private: + Action *pending_action = nullptr; +}; + +inline bool +PendingAction::empty() const +{ + return pending_action == nullptr; +} + +inline PendingAction & +PendingAction::operator=(Action *action) +{ + // Apparently HttpSM depends on not canceling the previous action if anew + // one completes immediately. Canceling the contained action in that case + // cause the HttpSm to permanently stall. + if (ACTION_RESULT_DONE != action) { + if (action != pending_action && pending_action != nullptr) { + pending_action->cancel(); + } + pending_action = action; + } + return *this; +} + +inline Continuation * +PendingAction::get_continuation() const +{ + return pending_action ? pending_action->continuation : nullptr; +} + +inline Action * +PendingAction::get() const +{ + return pending_action; +} + +inline PendingAction::~PendingAction() +{ + if (pending_action) { + pending_action->cancel(); + } +} + +inline void +PendingAction::clear_if_action_is(Action *action) +{ + if (action == pending_action) { + pending_action = nullptr; + } +} diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 76d7d0105..e03144c86 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -1564,7 +1564,7 @@ plugins required to work with sni_routing. if (!lock.is_locked()) { api_timer = -Thread::get_hrtime_updated(); HTTP_SM_SET_DEFAULT_HANDLER(&HttpSM::state_api_callout); - ink_release_assert(pending_action.is_empty()); + ink_release_assert(pending_action.empty()); pending_action = mutex->thread_holding->schedule_in(this, HRTIME_MSECONDS(10)); return -1; } @@ -1888,7 +1888,7 @@ HttpSM::state_http_server_open(int event, void *data) SMDebug("http_track", "entered inside state_http_server_open: %s", HttpDebugNames::get_event_name(event)); STATE_ENTER(&HttpSM::state_http_server_open, event); ink_release_assert(event == EVENT_INTERVAL || event == NET_EVENT_OPEN || event == NET_EVENT_OPEN_FAILED || - pending_action.is_empty()); + pending_action.empty()); if (event != NET_EVENT_OPEN) { pending_action = nullptr; } @@ -1911,7 +1911,7 @@ HttpSM::state_http_server_open(int event, void *data) // Since the UnixNetVConnection::action_ or SocksEntry::action_ may be returned from netProcessor.connect_re, and the // SocksEntry::action_ will be copied into UnixNetVConnection::action_ before call back NET_EVENT_OPEN from SocksEntry::free(), // so we just compare the Continuation between pending_action and VC's action_. - ink_release_assert(pending_action.is_empty() || pending_action.get_continuation() == vc->get_action()->continuation); + ink_release_assert(pending_action.empty() || pending_action.get_continuation() == vc->get_action()->continuation); pending_action = nullptr; if (this->plugin_tunnel_type == HTTP_NO_PLUGIN_TUNNEL) { @@ -2425,7 +2425,7 @@ HttpSM::state_hostdb_lookup(int event, void *data) opt.host_res_style = ats_host_res_from(ua_txn->get_netvc()->get_local_addr()->sa_family, t_state.txn_conf->host_res_data.order); pending_action = hostDBProcessor.getbyname_imm(this, (cb_process_result_pfn)&HttpSM::process_hostdb_info, host_name, 0, opt); - if (pending_action.is_empty()) { + if (pending_action.empty()) { call_transact_and_set_next_state(nullptr); } } break; @@ -2560,7 +2560,7 @@ HttpSM::state_cache_open_write(int event, void *data) // Make sure we are on the "right" thread if (ua_txn) { pending_action = ua_txn->adjust_thread(this, event, data); - if (!pending_action.is_empty()) { + if (!pending_action.empty()) { HTTP_INCREMENT_DYN_STAT(http_cache_open_write_adjust_thread_stat); return 0; // Go away if we reschedule } @@ -4282,7 +4282,7 @@ void HttpSM::do_hostdb_lookup() { ink_assert(t_state.dns_info.lookup_name != nullptr); - ink_assert(pending_action.is_empty()); + ink_assert(pending_action.empty()); milestones[TS_MILESTONE_DNS_LOOKUP_BEGIN] = Thread::get_hrtime(); @@ -4300,7 +4300,7 @@ HttpSM::do_hostdb_lookup() opt.timeout = t_state.api_txn_dns_timeout_value; } pending_action = hostDBProcessor.getSRVbyname_imm(this, (cb_process_result_pfn)&HttpSM::process_srv_info, d, 0, opt); - if (pending_action.is_empty()) { + if (pending_action.empty()) { char *host_name = t_state.dns_info.srv_lookup_success ? t_state.dns_info.srv_hostname : t_state.dns_info.lookup_name; opt.port = t_state.dns_info.srv_lookup_success ? t_state.dns_info.srv_port : @@ -4313,7 +4313,7 @@ HttpSM::do_hostdb_lookup() ats_host_res_from(ua_txn->get_netvc()->get_local_addr()->sa_family, t_state.txn_conf->host_res_data.order); pending_action = hostDBProcessor.getbyname_imm(this, (cb_process_result_pfn)&HttpSM::process_hostdb_info, host_name, 0, opt); - if (pending_action.is_empty()) { + if (pending_action.empty()) { call_transact_and_set_next_state(nullptr); } } @@ -4346,7 +4346,7 @@ HttpSM::do_hostdb_lookup() pending_action = hostDBProcessor.getbyname_imm(this, (cb_process_result_pfn)&HttpSM::process_hostdb_info, t_state.dns_info.lookup_name, 0, opt); - if (pending_action.is_empty()) { + if (pending_action.empty()) { call_transact_and_set_next_state(nullptr); } return; @@ -4359,7 +4359,7 @@ void HttpSM::do_hostdb_reverse_lookup() { ink_assert(t_state.dns_info.lookup_name != nullptr); - ink_assert(pending_action.is_empty()); + ink_assert(pending_action.empty()); SMDebug("http_seq", "Doing reverse DNS Lookup"); @@ -4799,7 +4799,7 @@ HttpSM::do_cache_lookup_and_read() { // TODO decide whether to uncomment after finish testing redirect // ink_assert(server_txn == NULL); - ink_assert(pending_action.is_empty()); + ink_assert(pending_action.empty()); t_state.request_sent_time = UNDEFINED_TIME; t_state.response_received_time = UNDEFINED_TIME; @@ -4898,7 +4898,7 @@ HttpSM::do_cache_prepare_action(HttpCacheSM *c_sm, CacheHTTPInfo *object_read_in URL *o_url, *s_url; bool restore_client_request = false; - ink_assert(pending_action.is_empty()); + ink_assert(pending_action.empty()); if (t_state.redirect_info.redirect_in_process) { o_url = &(t_state.redirect_info.original_url); @@ -5058,7 +5058,7 @@ HttpSM::do_http_server_open(bool raw) ink_assert(ua_entry != nullptr || t_state.req_flavor == HttpTransact::REQ_FLAVOR_SCHEDULED_UPDATE || t_state.req_flavor == HttpTransact::REQ_FLAVOR_REVPROXY); - ink_assert(pending_action.is_empty()); + ink_assert(pending_action.empty()); ink_assert(t_state.current.server->dst_addr.network_order_port() != 0); char addrbuf[INET6_ADDRPORTSTRLEN]; @@ -5290,7 +5290,7 @@ HttpSM::do_http_server_open(bool raw) if (ccount > t_state.txn_conf->outbound_conntrack.max) { ct_state.release(); - ink_assert(pending_action.is_empty()); // in case of reschedule must not have already pending. + ink_assert(pending_action.empty()); // in case of reschedule must not have already pending. // If the queue is disabled, reschedule. if (t_state.http_config_param->global_outbound_conntrack.queue_size < 0) { @@ -7193,10 +7193,10 @@ HttpSM::kill_this() // state. This is because we are depending on the // callout to complete for the state machine to // get killed. - if (callout_state == HTTP_API_NO_CALLOUT && !pending_action.is_empty()) { + if (callout_state == HTTP_API_NO_CALLOUT && !pending_action.empty()) { pending_action = nullptr; - } else if (!pending_action.is_empty()) { - ink_assert(pending_action.is_empty()); + } else if (!pending_action.empty()) { + ink_assert(pending_action.empty()); } cache_sm.end_both(); @@ -7294,7 +7294,7 @@ HttpSM::kill_this() plugin_tunnel = nullptr; } - ink_assert(pending_action.is_empty()); + ink_assert(pending_action.empty()); ink_release_assert(vc_table.is_table_clear() == true); ink_release_assert(tunnel.is_tunnel_active() == false); @@ -8293,7 +8293,7 @@ HttpSM::get_http_schedule(int event, void * /* data ATS_UNUSED */) if (!plugin_lock) { HTTP_SM_SET_DEFAULT_HANDLER(&HttpSM::get_http_schedule); - ink_assert(pending_action.is_empty()); + ink_assert(pending_action.empty()); pending_action = mutex->thread_holding->schedule_in(this, HRTIME_MSECONDS(10)); return 0; } else { diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h index 9dd737541..8a8664e79 100644 --- a/proxy/http/HttpSM.h +++ b/proxy/http/HttpSM.h @@ -44,6 +44,7 @@ #include "../ProxyTransaction.h" #include "HdrUtils.h" #include "tscore/History.h" +#include "tscore/PendingAction.h" #define HTTP_API_CONTINUE (INK_API_EVENT_EVENTS_START + 0) #define HTTP_API_ERROR (INK_API_EVENT_EVENTS_START + 1) @@ -168,54 +169,6 @@ enum HttpPluginTunnel_t { class PluginVCCore; -class PendingAction -{ -public: - bool - is_empty() const - { - return pending_action == nullptr; - } - PendingAction & - operator=(Action *b) - { - // Don't do anything if the new action is _DONE - if (b != ACTION_RESULT_DONE) { - if (b != pending_action && pending_action != nullptr) { - pending_action->cancel(); - } - pending_action = b; - } - return *this; - } - Continuation * - get_continuation() const - { - return pending_action ? pending_action->continuation : nullptr; - } - Action * - get() const - { - return pending_action; - } - void - clear_if_action_is(Action *current_action) - { - if (current_action == pending_action) { - pending_action = nullptr; - } - } - ~PendingAction() - { - if (pending_action) { - pending_action->cancel(); - } - } - -private: - Action *pending_action = nullptr; -}; - class PostDataBuffers { public:
