commit ffb7bd5a17789fa1b0961865b6e9b945f8babde2
parent 6fc34a866590231a6a9f8def970e297cf071b31d
Author: Kershaw Chang <kershaw@mozilla.com>
Date: Mon, 8 Dec 2025 16:56:49 +0000
Bug 1999691 - When a DNS cache entry is used, take it out and readd it to the eviction queue, r=necko-reviewers,valentin
Differential Revision: https://phabricator.services.mozilla.com/D275197
Diffstat:
7 files changed, 172 insertions(+), 13 deletions(-)
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -15659,6 +15659,13 @@
value: ""
mirror: never
+# When true, placing the most recent used cache entry
+# to the tail of the EvictionQ.
+- name: network.dns.mru_to_tail
+ type: RelaxedAtomicBool
+ value: @IS_NIGHTLY_BUILD@
+ mirror: always
+
# Whether to add additional record IPs to the cache
- name: network.trr.add_additional_records
type: RelaxedAtomicBool
diff --git a/netwerk/dns/HostRecordQueue.cpp b/netwerk/dns/HostRecordQueue.cpp
@@ -92,6 +92,21 @@ void HostRecordQueue::AddToEvictionQ(
}
}
+void HostRecordQueue::MoveToEvictionQueueTail(
+ nsHostRecord* aRec, const MutexAutoLock& aProofOfLock) {
+ bool inEvictionQ = mEvictionQ.contains(aRec);
+ if (!inEvictionQ) {
+ // Note: this function can be called when the record isn’t in the
+ // mEvictionQ. For example, if we immediately start a TTL lookup (see
+ // nsHostResolver::CompleteLookupLocked), the record may not be in
+ // mEvictionQ.
+ return;
+ }
+
+ aRec->remove();
+ mEvictionQ.insertBack(aRec);
+}
+
void HostRecordQueue::MaybeRenewHostRecord(nsHostRecord* aRec,
const MutexAutoLock& aProofOfLock) {
if (!aRec->isInList()) {
diff --git a/netwerk/dns/HostRecordQueue.h b/netwerk/dns/HostRecordQueue.h
@@ -34,6 +34,11 @@ class HostRecordQueue final {
nsHostRecord* aRec, uint32_t aMaxCacheEntries,
nsRefPtrHashtable<nsGenericHashKey<nsHostKey>, nsHostRecord>& aDB,
const MutexAutoLock& aProofOfLock);
+
+ // Move aRec to the tail of mEvictionQ (the most-recently-used end).
+ void MoveToEvictionQueueTail(nsHostRecord* aRec,
+ const MutexAutoLock& aProofOfLock);
+
// Called for removing the record from mEvictionQ. When this function is
// called, the record should be either in mEvictionQ or not in any queue.
void MaybeRenewHostRecord(nsHostRecord* aRec,
diff --git a/netwerk/dns/nsHostResolver.cpp b/netwerk/dns/nsHostResolver.cpp
@@ -591,7 +591,7 @@ nsresult nsHostResolver::ResolveHost(const nsACString& aHost,
} else if (!rec->mResolving) {
result =
FromUnspecEntry(rec, host, aTrrServer, originSuffix, type, flags, af,
- aOriginAttributes.IsPrivateBrowsing(), status);
+ aOriginAttributes.IsPrivateBrowsing(), status, lock);
// If this is a by-type request or if no valid record was found
// in the cache or this is an AF_UNSPEC request, then start a
// new lookup.
@@ -675,12 +675,14 @@ already_AddRefed<nsHostRecord> nsHostResolver::FromCache(
// but start a new lookup in the background.
//
// Also records telemetry for type of cache hit (HIT/NEGATIVE_HIT/RENEWAL).
- ConditionallyRefreshRecord(aRec, aHost, aLock);
+ bool refresh = ConditionallyRefreshRecord(aRec, aHost, aLock);
if (aRec->negative) {
LOG((" Negative cache entry for host [%s].\n",
nsPromiseFlatCString(aHost).get()));
aStatus = NS_ERROR_UNKNOWN_HOST;
+ } else if (StaticPrefs::network_dns_mru_to_tail() && !refresh) {
+ mQueue.MoveToEvictionQueueTail(aRec, aLock);
}
return result.forget();
@@ -707,7 +709,8 @@ already_AddRefed<nsHostRecord> nsHostResolver::FromIPLiteral(
already_AddRefed<nsHostRecord> nsHostResolver::FromUnspecEntry(
nsHostRecord* aRec, const nsACString& aHost, const nsACString& aTrrServer,
const nsACString& aOriginSuffix, uint16_t aType,
- nsIDNSService::DNSFlags aFlags, uint16_t af, bool aPb, nsresult& aStatus) {
+ nsIDNSService::DNSFlags aFlags, uint16_t af, bool aPb, nsresult& aStatus,
+ const MutexAutoLock& aLock) {
RefPtr<nsHostRecord> result = nullptr;
// If this is an IPV4 or IPV6 specific request, check if there is
// an AF_UNSPEC entry we can use. Otherwise, hit the resolver...
@@ -773,7 +776,10 @@ already_AddRefed<nsHostRecord> nsHostResolver::FromUnspecEntry(
if (aRec->negative) {
aStatus = NS_ERROR_UNKNOWN_HOST;
}
- ConditionallyRefreshRecord(aRec, aHost, lock);
+ bool refresh = ConditionallyRefreshRecord(aRec, aHost, lock);
+ if (!refresh) {
+ AddToEvictionQ(result, aLock);
+ }
} else if (af == PR_AF_INET6) {
// For AF_INET6, a new lookup means another AF_UNSPEC
// lookup. We have already iterated through the
@@ -1183,14 +1189,15 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec,
return rv;
}
-nsresult nsHostResolver::ConditionallyRefreshRecord(
- nsHostRecord* rec, const nsACString& host, const MutexAutoLock& aLock) {
+bool nsHostResolver::ConditionallyRefreshRecord(nsHostRecord* rec,
+ const nsACString& host,
+ const MutexAutoLock& aLock) {
if ((rec->CheckExpiration(TimeStamp::NowLoRes()) == nsHostRecord::EXP_GRACE ||
rec->negative) &&
!rec->mResolving && rec->RefreshForNegativeResponse()) {
LOG((" Using %s cache entry for host [%s] but starting async renewal.",
rec->negative ? "negative" : "positive", host.BeginReading()));
- NameLookup(rec, aLock);
+ nsresult rv = NameLookup(rec, aLock);
if (rec->IsAddrRecord()) {
if (!rec->negative) {
@@ -1199,6 +1206,8 @@ nsresult nsHostResolver::ConditionallyRefreshRecord(
glean::dns::lookup_method.AccumulateSingleSample(METHOD_NEGATIVE_HIT);
}
}
+
+ return NS_SUCCEEDED(rv);
} else if (rec->IsAddrRecord()) {
// it could be that the record is negative, but we haven't entered the above
// if condition due to the second expression being false. In that case we
@@ -1210,7 +1219,7 @@ nsresult nsHostResolver::ConditionallyRefreshRecord(
}
}
- return NS_OK;
+ return false;
}
bool nsHostResolver::GetHostToLookup(nsHostRecord** result) {
diff --git a/netwerk/dns/nsHostResolver.h b/netwerk/dns/nsHostResolver.h
@@ -234,12 +234,13 @@ class nsHostResolver : public nsISupports, public AHostResolver {
/**
* Starts a new lookup in the background for cached entries that are in the
- * grace period or that are are negative.
+ * grace period or that are are negative. Return true when the new lookup
+ * starts.
*
* Also records telemetry for type of cache hit (HIT/NEGATIVE_HIT/RENEWAL).
*/
- nsresult ConditionallyRefreshRecord(nsHostRecord* rec, const nsACString& host,
- const mozilla::MutexAutoLock& aLock)
+ bool ConditionallyRefreshRecord(nsHostRecord* rec, const nsACString& host,
+ const mozilla::MutexAutoLock& aLock)
MOZ_REQUIRES(mLock);
void OnResolveComplete(nsHostRecord* aRec,
@@ -266,8 +267,8 @@ class nsHostResolver : public nsISupports, public AHostResolver {
already_AddRefed<nsHostRecord> FromUnspecEntry(
nsHostRecord* aRec, const nsACString& aHost, const nsACString& aTrrServer,
const nsACString& aOriginSuffix, uint16_t aType,
- nsIDNSService::DNSFlags aFlags, uint16_t af, bool aPb, nsresult& aStatus)
- MOZ_REQUIRES(mLock);
+ nsIDNSService::DNSFlags aFlags, uint16_t af, bool aPb, nsresult& aStatus,
+ const mozilla::MutexAutoLock& aLock) MOZ_REQUIRES(mLock);
enum {
METHOD_HIT = 1,
diff --git a/netwerk/test/gtest/TestHostRecordQueue.cpp b/netwerk/test/gtest/TestHostRecordQueue.cpp
@@ -0,0 +1,120 @@
+#include "gtest/gtest.h"
+
+#include "HostRecordQueue.h"
+#include "TRRQuery.h"
+#include "TRR.h"
+
+using namespace mozilla;
+using namespace mozilla::net;
+
+class MockHostRecord : public nsHostRecord {
+ public:
+ NS_DECL_ISUPPORTS_INHERITED
+ explicit MockHostRecord(const nsHostKey& aKey) : nsHostRecord(aKey) {
+ negative = true;
+ }
+
+ void ResolveComplete() override {}
+
+ bool HasUsableResultInternal(
+ const mozilla::TimeStamp& now,
+ nsIDNSService::DNSFlags queryFlags) const override {
+ return true;
+ }
+
+ private:
+ ~MockHostRecord() = default;
+};
+
+NS_IMPL_ISUPPORTS_INHERITED(MockHostRecord, nsHostRecord, MockHostRecord)
+
+class HostRecordQueueTest : public ::testing::Test {
+ protected:
+ void SetUp() override {}
+
+ HostRecordQueue queue;
+ Mutex mMutex{"HostRecordQueueTest"};
+ nsRefPtrHashtable<nsGenericHashKey<nsHostKey>, nsHostRecord> mDB;
+};
+
+static RefPtr<nsHostRecord> CreateAndInsertMockRecord(
+ nsRefPtrHashtable<nsGenericHashKey<nsHostKey>, nsHostRecord>& aDB,
+ const char* hostName) {
+ nsHostKey key(nsCString(hostName), ""_ns, 0,
+ nsIDNSService::RESOLVE_DEFAULT_FLAGS, 0, false, ""_ns);
+ return aDB.LookupOrInsertWith(key, [&] { return new MockHostRecord(key); });
+}
+
+TEST_F(HostRecordQueueTest, AddToEvictionQ_BelowMax) {
+ RefPtr<nsHostRecord> rec = CreateAndInsertMockRecord(mDB, "A.com");
+
+ MutexAutoLock lock(mMutex);
+
+ queue.AddToEvictionQ(rec.get(), 100, mDB, lock);
+
+ ASSERT_EQ(1u, queue.EvictionQSize());
+ ASSERT_TRUE(rec->isInList());
+
+ // Cleanup
+ rec->remove();
+}
+
+// When the eviction queue is at capacity, adding a new entry evicts the
+// oldest (head) entry.
+TEST_F(HostRecordQueueTest, AddToEvictionQ_AtMax_EvictsOldest) {
+ const uint32_t MAX_ENTRIES = 3;
+
+ RefPtr<nsHostRecord> rec1 = CreateAndInsertMockRecord(mDB, "A.com");
+ RefPtr<nsHostRecord> rec2 = CreateAndInsertMockRecord(mDB, "B.com");
+ RefPtr<nsHostRecord> rec3 = CreateAndInsertMockRecord(mDB, "C.com");
+
+ MutexAutoLock lock(mMutex);
+ queue.AddToEvictionQ(rec1, MAX_ENTRIES, mDB, lock);
+ queue.AddToEvictionQ(rec2, MAX_ENTRIES, mDB, lock);
+ queue.AddToEvictionQ(rec3, MAX_ENTRIES, mDB, lock);
+
+ ASSERT_EQ(MAX_ENTRIES, queue.EvictionQSize());
+
+ RefPtr<nsHostRecord> rec4 = CreateAndInsertMockRecord(mDB, "New.com");
+ queue.AddToEvictionQ(rec4, MAX_ENTRIES, mDB, lock);
+
+ ASSERT_TRUE(rec2->isInList());
+ ASSERT_TRUE(rec3->isInList());
+ ASSERT_TRUE(rec4->isInList());
+ ASSERT_FALSE(rec1->isInList());
+
+ rec2->remove();
+ rec3->remove();
+ rec4->remove();
+}
+
+// After adding a new record, the touched entry
+// remains, and the oldest untouched entry is evicted.
+TEST_F(HostRecordQueueTest, MoveToEvictionQueueTail) {
+ const uint32_t MAX_ENTRIES = 3;
+
+ RefPtr<nsHostRecord> rec1 = CreateAndInsertMockRecord(mDB, "A.com");
+ RefPtr<nsHostRecord> rec2 = CreateAndInsertMockRecord(mDB, "B.com");
+ RefPtr<nsHostRecord> rec3 = CreateAndInsertMockRecord(mDB, "C.com");
+
+ MutexAutoLock lock(mMutex);
+ queue.AddToEvictionQ(rec1, MAX_ENTRIES, mDB, lock);
+ queue.AddToEvictionQ(rec2, MAX_ENTRIES, mDB, lock);
+ queue.AddToEvictionQ(rec3, MAX_ENTRIES, mDB, lock);
+
+ ASSERT_EQ(MAX_ENTRIES, queue.EvictionQSize());
+
+ queue.MoveToEvictionQueueTail(rec1, lock);
+
+ RefPtr<nsHostRecord> rec4 = CreateAndInsertMockRecord(mDB, "New.com");
+ queue.AddToEvictionQ(rec4, MAX_ENTRIES, mDB, lock);
+
+ ASSERT_TRUE(rec1->isInList());
+ ASSERT_TRUE(rec3->isInList());
+ ASSERT_TRUE(rec4->isInList());
+ ASSERT_FALSE(rec2->isInList());
+
+ rec1->remove();
+ rec3->remove();
+ rec4->remove();
+}
diff --git a/netwerk/test/gtest/moz.build b/netwerk/test/gtest/moz.build
@@ -16,6 +16,7 @@ UNIFIED_SOURCES += [
"TestDNSPacket.cpp",
"TestEtcHostsParsing.cpp",
"TestHeaders.cpp",
+ "TestHostRecordQueue.cpp",
"TestHttp2WebTransport.cpp",
"TestHttp3ConnectUDPStream.cpp",
"TestHttpAtom.cpp",
@@ -69,6 +70,7 @@ LOCAL_INCLUDES += [
"/netwerk/base",
"/netwerk/cache2",
"/netwerk/cookie",
+ "/netwerk/dns",
"/netwerk/protocol/http",
"/toolkit/components/jsoncpp/include",
"/xpcom/tests/gtest",
