You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/05/16 15:27:56 UTC
[trafficserver] branch 9.2.x updated (d9186e76c -> b54edeaf5)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a change to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
from d9186e76c Fixes leak in SNIAction name globbing (#8827)
new 353cedbbd Promote class PendingAction from HttpSM.h for use in other classes. (#8423)
new b54edeaf5 Add `#pragma once` for PendingAction.h (#8846)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
include/tscore/PendingAction.h | 131 +++++++++++++++++++++++++++++++++++++++++
proxy/http/HttpSM.cc | 38 ++++++------
proxy/http/HttpSM.h | 49 +--------------
3 files changed, 151 insertions(+), 67 deletions(-)
create mode 100644 include/tscore/PendingAction.h
[trafficserver] 02/02: Add `#pragma once` for PendingAction.h (#8846)
Posted by zw...@apache.org.
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 b54edeaf5afb48cee2110501a2bd2ca2a8c40d52
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Thu May 12 16:34:52 2022 -0500
Add `#pragma once` for PendingAction.h (#8846)
PendingAction.h needs a `#pragma once` to protect it from double
inclusion. This patch adds this. This has not caused problems yet
because it is currently only used once in HttpSM.h, but as it gets more
use this will be required.
(cherry picked from commit 214b89261396f11c0866ce5f934ba24ca53ecde2)
---
include/tscore/PendingAction.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/tscore/PendingAction.h b/include/tscore/PendingAction.h
index c23bb9cb6..d1112f433 100644
--- a/include/tscore/PendingAction.h
+++ b/include/tscore/PendingAction.h
@@ -21,6 +21,8 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
+#pragma once
+
/** Hold a pending @c Action.
*
* This is modeled on smart pointer classes. This class wraps a pointer to
[trafficserver] 01/02: Promote class PendingAction from HttpSM.h for use in other classes. (#8423)
Posted by zw...@apache.org.
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 <am...@apache.org>
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: