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 2015/07/29 01:40:23 UTC
[45/62] [abbrv] trafficserver git commit: TS-3774: Fix memory leak
issue in host file parsing for HostDB.
TS-3774: Fix memory leak issue in host file parsing for HostDB.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/c36204d5
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/c36204d5
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/c36204d5
Branch: refs/heads/6.0.x
Commit: c36204d5abaf3d4c335dd885ace1e5d58b57d451
Parents: 3838cf2
Author: Alan M. Carroll <am...@apache.org>
Authored: Tue Jul 21 11:10:05 2015 -0500
Committer: Alan M. Carroll <am...@apache.org>
Committed: Thu Jul 23 15:43:50 2015 -0500
----------------------------------------------------------------------
iocore/hostdb/HostDB.cc | 39 +++++++++++++++++++---------------
iocore/hostdb/I_HostDBProcessor.h | 7 +++++-
iocore/hostdb/P_HostDBProcessor.h | 4 +++-
3 files changed, 31 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c36204d5/iocore/hostdb/HostDB.cc
----------------------------------------------------------------------
diff --git a/iocore/hostdb/HostDB.cc b/iocore/hostdb/HostDB.cc
index bb438a1..0df0cd8 100644
--- a/iocore/hostdb/HostDB.cc
+++ b/iocore/hostdb/HostDB.cc
@@ -662,19 +662,13 @@ HostDBInfo *
probe(ProxyMutex *mutex, HostDBMD5 const &md5, bool ignore_timeout)
{
// If we have an entry in our hosts file, we don't need to bother with DNS
+ // Make a local copy/reference so we don't have to worry about updates.
Ptr<RefCountedHostsFileMap> current_host_file_map = hostDB.hosts_file_ptr;
ts::ConstBuffer hname(md5.host_name, md5.host_len);
HostsFileMap::iterator find_result = current_host_file_map->hosts_file_map.find(hname);
if (find_result != current_host_file_map->hosts_file_map.end()) {
- // TODO: Something to make this not local :/
- static HostDBInfo r;
- r.round_robin = false;
- r.round_robin_elt = false;
- r.reverse_dns = false;
- r.is_srv = false;
- ats_ip_set(r.ip(), (*find_result).second);
- return &r;
+ return &(find_result->second);
}
ink_assert(this_ethread() == hostDB.lock_for_bucket((int)(fold_md5(md5.hash) % hostDB.buckets))->thread_holding);
@@ -2731,8 +2725,6 @@ HostDBFileContinuation::destroy()
// proceeding at a time in any case so we might as well make these
// globals.
int HostDBFileUpdateActive = 0;
-// map of hostname -> IpAddr
-Ptr<RefCountedHostsFileMap> parsed_hosts_file_ptr(new RefCountedHostsFileMap());
// Actual ordering doesn't matter as long as it's consistent.
bool
@@ -2743,7 +2735,7 @@ CmpMD5(INK_MD5 const &lhs, INK_MD5 const &rhs)
void
-ParseHostLine(char *l)
+ParseHostLine(RefCountedHostsFileMap *map, char *l)
{
Tokenizer elts(" \t");
int n_elts = elts.Initialize(l, SHARE_TOKS);
@@ -2754,8 +2746,13 @@ ParseHostLine(char *l)
for (int i = 1; i < n_elts; ++i) {
ts::ConstBuffer name(elts[i], strlen(elts[i]));
// If we don't have an entry already (host files only support single IPs for a given name)
- if (parsed_hosts_file_ptr->hosts_file_map.find(name) == parsed_hosts_file_ptr->hosts_file_map.end()) {
- parsed_hosts_file_ptr->hosts_file_map[name] = ip;
+ if (map->hosts_file_map.find(name) == map->hosts_file_map.end()) {
+ HostsFileMap::mapped_type& item = map->hosts_file_map[name];
+ item.round_robin = false;
+ item.round_robin_elt = false;
+ item.reverse_dns = false;
+ item.is_srv = false;
+ ats_ip_set(item.ip(), ip);
}
}
}
@@ -2765,6 +2762,8 @@ ParseHostLine(char *l)
void
ParseHostFile(char const *path)
{
+ Ptr<RefCountedHostsFileMap> parsed_hosts_file_ptr;
+
// Test and set for update in progress.
if (0 != ink_atomic_swap(&HostDBFileUpdateActive, 1)) {
Debug("hostdb", "Skipped load of host file because update already in progress");
@@ -2780,6 +2779,7 @@ ParseHostFile(char const *path)
// +1 in case no terminating newline
int64_t size = info.st_size + 1;
+ parsed_hosts_file_ptr = new RefCountedHostsFileMap;
parsed_hosts_file_ptr->HostFileText = static_cast<char *>(ats_malloc(size));
if (parsed_hosts_file_ptr->HostFileText) {
char *base = parsed_hosts_file_ptr->HostFileText;
@@ -2805,7 +2805,7 @@ ParseHostFile(char const *path)
while (base < spot && isspace(*base))
++base; // skip leading ws
if (*base != '#' && base < spot) // non-empty non-comment line
- ParseHostLine(base);
+ ParseHostLine(parsed_hosts_file_ptr, base);
base = spot + 1;
}
@@ -2815,10 +2815,15 @@ ParseHostFile(char const *path)
}
}
- // Swap out hostDB's map for ours
+ // Rotate the host file maps down. We depend on two assumptions here -
+ // 1) Host files are not updated frequently. Even if updated via traffic_ctl that will be at least 5-10 seconds between reloads.
+ // 2) The HostDB clients (essentially the HttpSM) copies the HostDB record or data over to local storage during event processing,
+ // which means the data only has to be valid until the event chaining rolls back up to the event processor. This will certainly
+ // be less than 1s
+ // The combination of these means keeping one file back in the rotation is sufficient to keep any outstanding references valid until
+ // dropped.
+ hostDB.prev_hosts_file_ptr = hostDB.hosts_file_ptr;
hostDB.hosts_file_ptr = parsed_hosts_file_ptr;
- // Make a new map, so we can do it all again
- parsed_hosts_file_ptr = new RefCountedHostsFileMap();
// Mark this one as completed, so we can allow another update to happen
HostDBFileUpdateActive = 0;
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c36204d5/iocore/hostdb/I_HostDBProcessor.h
----------------------------------------------------------------------
diff --git a/iocore/hostdb/I_HostDBProcessor.h b/iocore/hostdb/I_HostDBProcessor.h
index abb0997..abbbc55 100644
--- a/iocore/hostdb/I_HostDBProcessor.h
+++ b/iocore/hostdb/I_HostDBProcessor.h
@@ -157,12 +157,17 @@ struct HostDBInfo {
char *hostname();
char *srvname(HostDBRoundRobin *rr);
- /// Check if this entry is a round robin entry.
+ /// Check if this entry is the root of a round robin entry.
+ /// If @c true then this entry needs to be converted to a specific element of the round robin to be used.
bool
is_rr() const
{
return 0 != round_robin;
}
+ /// Check if this entry is an element of a round robin entry.
+ /// If @c true then this entry is part of and was obtained from a round robin root. This is useful if the
+ /// address doesn't work - a retry can probably get a new address by doing another lookup and resolving to
+ /// a different element of the round robin.
bool
is_rr_elt() const
{
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c36204d5/iocore/hostdb/P_HostDBProcessor.h
----------------------------------------------------------------------
diff --git a/iocore/hostdb/P_HostDBProcessor.h b/iocore/hostdb/P_HostDBProcessor.h
index 5cfa025..22b095d 100644
--- a/iocore/hostdb/P_HostDBProcessor.h
+++ b/iocore/hostdb/P_HostDBProcessor.h
@@ -196,7 +196,7 @@ struct CmpConstBuffferCaseInsensitive {
};
// Our own typedef for the host file mapping
-typedef std::map<ts::ConstBuffer, IpAddr, CmpConstBuffferCaseInsensitive> HostsFileMap;
+typedef std::map<ts::ConstBuffer, HostDBInfo, CmpConstBuffferCaseInsensitive> HostsFileMap;
// A to hold a ref-counted map
struct RefCountedHostsFileMap : public RefCountObj {
HostsFileMap hosts_file_map;
@@ -225,6 +225,8 @@ struct HostDBCache : public MultiCache<HostDBInfo> {
// Map to contain all of the host file overrides, initialize it to empty
Ptr<RefCountedHostsFileMap> hosts_file_ptr;
+ // Double buffer the hosts file becase it's small and it solves dangling reference problems.
+ Ptr<RefCountedHostsFileMap> prev_hosts_file_ptr;
Queue<HostDBContinuation, Continuation::Link_link> pending_dns[MULTI_CACHE_PARTITIONS];
Queue<HostDBContinuation, Continuation::Link_link> &pending_dns_for_hash(INK_MD5 &md5);