This is an automated email from the ASF dual-hosted git repository.
bneradt 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 f1827d453a Prevent HttpSM to be added to wrong ConnectingEntry (#10576)
f1827d453a is described below
commit f1827d453a735b7ab189bb6501475f8767714092
Author: Zhengxi Li <[email protected]>
AuthorDate: Mon Oct 9 19:09:00 2023 -0400
Prevent HttpSM to be added to wrong ConnectingEntry (#10576)
This PR fixes #10455, a crash observed in production coincided with changes
in #10157. Please see details in the issue.
---
proxy/http/ConnectingEntry.cc | 7 +++---
proxy/http/HttpSM.cc | 55 ++++++++++++++++++++-----------------------
2 files changed, 29 insertions(+), 33 deletions(-)
diff --git a/proxy/http/ConnectingEntry.cc b/proxy/http/ConnectingEntry.cc
index e0276ff09e..a06e9d2b34 100644
--- a/proxy/http/ConnectingEntry.cc
+++ b/proxy/http/ConnectingEntry.cc
@@ -152,13 +152,12 @@ ConnectingEntry::state_http_server_open(int event, void
*data)
void
ConnectingEntry::remove_entry()
{
- EThread *ethread = this_ethread();
- auto ip_iter = ethread->connecting_pool->m_ip_pool.find(this->ipaddr);
- while (ip_iter != ethread->connecting_pool->m_ip_pool.end() && this->ipaddr
== ip_iter->first) {
+ EThread *ethread = this_ethread();
+ auto [iter_start, iter_end] =
ethread->connecting_pool->m_ip_pool.equal_range(this->ipaddr);
+ for (auto ip_iter = iter_start; ip_iter != iter_end; ++ip_iter) {
if (ip_iter->second == this) {
ethread->connecting_pool->m_ip_pool.erase(ip_iter);
break;
}
- ++ip_iter;
}
}
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 8b2cf65a1e..055aca6f97 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -2143,33 +2143,30 @@ void
HttpSM::cancel_pending_server_connection()
{
EThread *ethread = this_ethread();
- if (nullptr == ethread->connecting_pool) {
+ if (nullptr == ethread->connecting_pool || !t_state.current.server) {
return; // No pending requests
}
- if (t_state.current.server) {
- IpEndpoint ip;
- ip.assign(&this->t_state.current.server->dst_addr.sa);
- auto ip_iter = ethread->connecting_pool->m_ip_pool.find(ip);
- while (ip_iter != ethread->connecting_pool->m_ip_pool.end() &&
ip_iter->first == ip) {
- ConnectingEntry *connecting_entry = ip_iter->second;
- // Found a match
- // Look for our sm in the queue
- auto entry = connecting_entry->connect_sms.find(this);
- if (entry != connecting_entry->connect_sms.end()) {
- connecting_entry->connect_sms.erase(entry);
- if (connecting_entry->connect_sms.empty()) {
- if (connecting_entry->netvc) {
- connecting_entry->netvc->do_io_write(nullptr, 0, nullptr);
- connecting_entry->netvc->do_io_close();
- }
- ethread->connecting_pool->m_ip_pool.erase(ip_iter);
- delete connecting_entry;
- break;
- } else {
- // Leave the shared entry remaining alone
+ IpEndpoint ip;
+ ip.assign(&this->t_state.current.server->dst_addr.sa);
+ auto [iter_start, iter_end] =
ethread->connecting_pool->m_ip_pool.equal_range(ip);
+ for (auto ip_iter = iter_start; ip_iter != iter_end; ++ip_iter) {
+ ConnectingEntry *connecting_entry = ip_iter->second;
+ // Found a match, look for our sm in the queue.
+ auto entry = connecting_entry->connect_sms.find(this);
+ if (entry != connecting_entry->connect_sms.end()) {
+ // Found the sm, remove it.
+ connecting_entry->connect_sms.erase(entry);
+ if (connecting_entry->connect_sms.empty()) {
+ if (connecting_entry->netvc) {
+ connecting_entry->netvc->do_io_write(nullptr, 0, nullptr);
+ connecting_entry->netvc->do_io_close();
}
+ ethread->connecting_pool->m_ip_pool.erase(ip_iter);
+ delete connecting_entry;
+ break;
+ } else {
+ // Leave the shared entry remaining alone
}
- ++ip_iter;
}
}
}
@@ -2198,23 +2195,23 @@ HttpSM::add_to_existing_request()
IpEndpoint ip;
ip.assign(&s.current.server->dst_addr.sa);
- auto ip_iter =
ethread->connecting_pool->m_ip_pool.find(ip);
std::string_view proposed_sni = this->get_outbound_sni();
std::string_view proposed_cert = this->get_outbound_cert();
std::string_view proposed_hostname = this->t_state.current.server->name;
- while (!retval && ip_iter != ethread->connecting_pool->m_ip_pool.end() &&
ip_iter->first == ip) {
- // Check that entry matches sni, hostname, and cert
+ auto [iter_start, iter_end] =
ethread->connecting_pool->m_ip_pool.equal_range(ip);
+
+ for (auto ip_iter = iter_start; ip_iter != iter_end; ++ip_iter) {
+ // Check that entry matches sni, hostname, and cert.
if (proposed_hostname == ip_iter->second->hostname && proposed_sni ==
ip_iter->second->sni &&
proposed_cert == ip_iter->second->cert_name &&
ip_iter->second->connect_sms.size() < 50) {
- // Pre-emptively set a server connect failure that will be cleared once
a WRITE_READY is received from origin or
- // bytes are received back
+ // Pre-emptively set a server connect failure that will be cleared once a
+ // WRITE_READY is received from origin or bytes are received back.
this->t_state.set_connect_fail(EIO);
ip_iter->second->connect_sms.insert(this);
Debug("http_connect", "Add entry to connection queue. size=%zd",
ip_iter->second->connect_sms.size());
retval = true;
break;
}
- ++ip_iter;
}
return retval;
}
