commit 828447d537c234bc52baecc51191aa4ada436fa9
parent b758ee7527ef5d8007b522f039d10885318df91b
Author: Atila Butkovits <abutkovits@mozilla.com>
Date: Mon, 5 Jan 2026 12:00:43 +0200
Revert "Bug 1999691 - When a DNS cache entry is used, take it out and readd it to the eviction queue, r=necko-reviewers,valentin" for causing high frequency failures at HostRecordQueue.cpp
This reverts commit 76a1a1d891b081c6274883f9b6cb3e86d2658b78.
Diffstat:
7 files changed, 5 insertions(+), 183 deletions(-)
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -15622,13 +15622,6 @@
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,22 +38,11 @@ void HostRecordQueue::InsertRecord(nsHostRecord* aRec,
mPendingCount++;
}
-void HostRecordQueue::MaybeAddToEvictionQ(
+void HostRecordQueue::AddToEvictionQ(
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);
@@ -103,21 +92,6 @@ 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,17 +33,7 @@ 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, lock);
+ aOriginAttributes.IsPrivateBrowsing(), status);
// 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,8 +687,6 @@ 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();
@@ -715,8 +713,7 @@ 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,
- const MutexAutoLock& aLock) {
+ nsIDNSService::DNSFlags aFlags, uint16_t af, bool aPb, nsresult& aStatus) {
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...
@@ -783,7 +780,6 @@ 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
@@ -1352,11 +1348,6 @@ 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,10 +246,6 @@ 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);
@@ -270,8 +266,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,
- const mozilla::MutexAutoLock& aLock) MOZ_REQUIRES(mLock);
+ nsIDNSService::DNSFlags aFlags, uint16_t af, bool aPb, nsresult& aStatus)
+ MOZ_REQUIRES(mLock);
enum {
METHOD_HIT = 1,
diff --git a/netwerk/test/gtest/TestHostRecordQueue.cpp b/netwerk/test/gtest/TestHostRecordQueue.cpp
@@ -1,120 +0,0 @@
-#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,7 +16,6 @@ UNIFIED_SOURCES += [
"TestDNSPacket.cpp",
"TestEtcHostsParsing.cpp",
"TestHeaders.cpp",
- "TestHostRecordQueue.cpp",
"TestHttp2WebTransport.cpp",
"TestHttp3ConnectUDPStream.cpp",
"TestHttpAtom.cpp",
@@ -70,7 +69,6 @@ LOCAL_INCLUDES += [
"/netwerk/base",
"/netwerk/cache2",
"/netwerk/cookie",
- "/netwerk/dns",
"/netwerk/protocol/http",
"/toolkit/components/jsoncpp/include",
"/xpcom/tests/gtest",
