commit 76a1a1d891b081c6274883f9b6cb3e86d2658b78
parent 877cf3739239e00baf074f0282f85c2fcc8ba721
Author: Kershaw Chang <kershaw@mozilla.com>
Date: Mon, 5 Jan 2026 08:48:11 +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, 183 insertions(+), 5 deletions(-)
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -15622,6 +15622,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
@@ -38,11 +38,22 @@ void HostRecordQueue::InsertRecord(nsHostRecord* aRec,
mPendingCount++;
}
-void HostRecordQueue::AddToEvictionQ(
+void HostRecordQueue::MaybeAddToEvictionQ(
nsHostRecord* aRec, uint32_t aMaxCacheEntries,
nsRefPtrHashtable<nsGenericHashKey<nsHostKey>, nsHostRecord>& aDB,
const MutexAutoLock& aProofOfLock) {
if (aRec->isInList()) {
+ return;
+ }
+
+ AddToEvictionQ(aRec, aMaxCacheEntries, aDB, aProofOfLock, true);
+}
+
+void HostRecordQueue::AddToEvictionQ(
+ nsHostRecord* aRec, uint32_t aMaxCacheEntries,
+ nsRefPtrHashtable<nsGenericHashKey<nsHostKey>, nsHostRecord>& aDB,
+ const MutexAutoLock& aProofOfLock, bool aSkipCheck) {
+ if (aRec->isInList() && !aSkipCheck) {
bool inEvictionQ = mEvictionQ.contains(aRec);
MOZ_DIAGNOSTIC_ASSERT(!inEvictionQ, "Already in eviction queue");
bool inHighQ = mHighQ.contains(aRec);
@@ -92,6 +103,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
@@ -33,7 +33,17 @@ class HostRecordQueue final {
void AddToEvictionQ(
nsHostRecord* aRec, uint32_t aMaxCacheEntries,
nsRefPtrHashtable<nsGenericHashKey<nsHostKey>, nsHostRecord>& aDB,
+ const MutexAutoLock& aProofOfLock, bool aSkipCheck = false);
+
+ void MaybeAddToEvictionQ(
+ 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
@@ -597,7 +597,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.
@@ -687,6 +687,8 @@ already_AddRefed<nsHostRecord> nsHostResolver::FromCache(
LOG((" Negative cache entry for host [%s].\n",
nsPromiseFlatCString(aHost).get()));
aStatus = NS_ERROR_UNKNOWN_HOST;
+ } else if (StaticPrefs::network_dns_mru_to_tail()) {
+ mQueue.MoveToEvictionQueueTail(aRec, aLock);
}
return result.forget();
@@ -713,7 +715,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...
@@ -780,6 +783,7 @@ already_AddRefed<nsHostRecord> nsHostResolver::FromUnspecEntry(
aStatus = NS_ERROR_UNKNOWN_HOST;
}
ConditionallyRefreshRecord(aRec, aHost, lock);
+ MaybeAddToEvictionQ(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
@@ -1348,6 +1352,11 @@ static bool different_rrset(AddrInfo* rrset1, AddrInfo* rrset2) {
return !eq;
}
+void nsHostResolver::MaybeAddToEvictionQ(nsHostRecord* rec,
+ const mozilla::MutexAutoLock& aLock) {
+ mQueue.MaybeAddToEvictionQ(rec, StaticPrefs::network_dnsCacheEntries(),
+ mRecordDB, aLock);
+}
void nsHostResolver::AddToEvictionQ(nsHostRecord* rec,
const MutexAutoLock& aLock) {
mQueue.AddToEvictionQ(rec, StaticPrefs::network_dnsCacheEntries(), mRecordDB,
diff --git a/netwerk/dns/nsHostResolver.h b/netwerk/dns/nsHostResolver.h
@@ -246,6 +246,10 @@ class nsHostResolver : public nsISupports, public AHostResolver {
const mozilla::MutexAutoLock& aLock)
MOZ_REQUIRES(mLock);
+ void MaybeAddToEvictionQ(nsHostRecord* rec,
+ const mozilla::MutexAutoLock& aLock)
+ MOZ_REQUIRES(mLock);
+
void AddToEvictionQ(nsHostRecord* rec, const mozilla::MutexAutoLock& aLock)
MOZ_REQUIRES(mLock);
@@ -266,8 +270,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",
