tor-browser

commit 4e7db39bb33668665ac2c2571ebef16ea2e39ad2
parent 3232d2a2c6e0ebeae7f494b3fd9e4c4ca7fa5dec
Author: Randell Jesup <rjesup@mozilla.com>
Date:   Thu, 18 Dec 2025 01:00:27 +0000

Bug 2004872: Clean up DNS code r=necko-reviewers,valentin

Differential Revision: https://phabricator.services.mozilla.com/D275557

Diffstat:
M
netwerk/dns/nsDNSService2.cpp
 | 
19
+++++++++++++++----
M
netwerk/dns/nsDNSService2.h
 | 
20
+++++++++++---------

2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/netwerk/dns/nsDNSService2.cpp b/netwerk/dns/nsDNSService2.cpp
@@ -666,11 +666,15 @@ NS_IMPL_ISUPPORTS(DNSServiceWrapper, nsIDNSService, nsPIDNSService)
 already_AddRefed<nsIDNSService> DNSServiceWrapper::GetSingleton() {
   if (!gDNSServiceWrapper) {
     gDNSServiceWrapper = new DNSServiceWrapper();
+    // Not strictly needed, but simple and avoids bypassing lock-checking
+    MutexAutoLock lock(gDNSServiceWrapper->mLock);
     gDNSServiceWrapper->mDNSServiceInUse = ChildDNSService::GetSingleton();
     if (gDNSServiceWrapper->mDNSServiceInUse) {
       ClearOnShutdown(&gDNSServiceWrapper);
       nsDNSPrefetch::Initialize(gDNSServiceWrapper);
     } else {
+      MutexAutoUnlock unlock(
+          gDNSServiceWrapper->mLock);  // don't destroy with held lock
       gDNSServiceWrapper = nullptr;
     }
   }
@@ -716,6 +720,7 @@ NS_IMPL_ISUPPORTS_INHERITED(nsDNSService, DNSServiceBase, nsIDNSService,
 static StaticRefPtr<nsDNSService> gDNSService;
 static Atomic<bool> gInited(false);
 
+// Note: be careful of races!  Called from multiple threads
 already_AddRefed<nsIDNSService> GetOrInitDNSService() {
   if (gInited) {
     return nsDNSService::GetXPCOMSingleton();
@@ -818,6 +823,7 @@ void nsDNSService::ReadPrefs(const char* name) {
     }
   }
   if (!name || !strcmp(name, kPrefIPv4OnlyDomains)) {
+    MutexAutoLock lock(mLock);
     Preferences::GetCString(kPrefIPv4OnlyDomains, mIPv4OnlyDomains);
   }
   if (!name || !strcmp(name, kPrefDnsLocalDomains)) {
@@ -852,7 +858,6 @@ void nsDNSService::ReadPrefs(const char* name) {
 
 NS_IMETHODIMP
 nsDNSService::Init() {
-  MOZ_ASSERT(!mResolver);
   MOZ_ASSERT(NS_IsMainThread());
 
   ReadPrefs(nullptr);
@@ -870,6 +875,7 @@ nsDNSService::Init() {
   if (NS_SUCCEEDED(rv)) {
     // now, set all of our member variables while holding the lock
     MutexAutoLock lock(mLock);
+    MOZ_ASSERT(!mResolver);
     mResolver = res;
   }
 
@@ -895,7 +901,12 @@ nsDNSService::Init() {
       do_GetService("@mozilla.org/network/oblivious-http-service;1"));
 
   mTrrService = new TRRService();
-  if (NS_FAILED(mTrrService->Init(mResolver->IsNativeHTTPSEnabled()))) {
+  bool httpsEnabled;
+  {
+    MutexAutoLock lock(mLock);
+    httpsEnabled = mResolver->IsNativeHTTPSEnabled();
+  }
+  if (NS_FAILED(mTrrService->Init(httpsEnabled))) {
     mTrrService = nullptr;
   }
 
@@ -1531,14 +1542,14 @@ nsresult nsDNSService::GetTRRDomainKey(nsACString& aTRRDomain) {
   return NS_OK;
 }
 
-size_t nsDNSService::SizeOfIncludingThis(
-    mozilla::MallocSizeOf mallocSizeOf) const {
+size_t nsDNSService::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) {
   // Measurement of the following members may be added later if DMD finds it
   // is worthwhile:
   // - mIDN
   // - mLock
 
   size_t n = mallocSizeOf(this);
+  MutexAutoLock lock(mLock);
   n += mResolver ? mResolver->SizeOfIncludingThis(mallocSizeOf) : 0;
   n += mIPv4OnlyDomains.SizeOfExcludingThisIfUnshared(mallocSizeOf);
   n += mLocalDomains.SizeOfExcludingThis(mallocSizeOf);
diff --git a/netwerk/dns/nsDNSService2.h b/netwerk/dns/nsDNSService2.h
@@ -40,7 +40,7 @@ class DNSServiceWrapper final : public nsPIDNSService {
   nsPIDNSService* PIDNSService();
 
   mozilla::Mutex mLock{"DNSServiceWrapper.mLock"};
-  nsCOMPtr<nsIDNSService> mDNSServiceInUse;
+  nsCOMPtr<nsIDNSService> mDNSServiceInUse MOZ_GUARDED_BY(mLock);
   nsCOMPtr<nsIDNSService> mBackupDNSService;
 };
 
@@ -58,7 +58,7 @@ class nsDNSService final : public mozilla::net::DNSServiceBase,
 
   static already_AddRefed<nsIDNSService> GetXPCOMSingleton();
 
-  size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
+  size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
   bool GetOffline() const;
 
@@ -83,7 +83,7 @@ class nsDNSService final : public mozilla::net::DNSServiceBase,
   nsresult PreprocessHostname(bool aLocalDomain, const nsACString& aInput,
                               nsACString& aACE);
 
-  bool IsLocalDomain(const nsACString& aHostname) const;
+  bool IsLocalDomain(const nsACString& aHostname) const MOZ_REQUIRES(mLock);
 
   nsresult AsyncResolveInternal(
       const nsACString& aHostname, uint16_t type, nsIDNSService::DNSFlags flags,
@@ -106,25 +106,27 @@ class nsDNSService final : public mozilla::net::DNSServiceBase,
   // Locks the mutex and returns an addreffed resolver. May return null.
   already_AddRefed<nsHostResolver> GetResolverLocked();
 
-  RefPtr<nsHostResolver> mResolver;
+  RefPtr<nsHostResolver> mResolver MOZ_GUARDED_BY(mLock);
 
   // mLock protects access to mResolver, mLocalDomains, mIPv4OnlyDomains,
   // mFailedSVCDomainNames, and mMockHTTPSRRDomain.
-  mozilla::Mutex mLock MOZ_UNANNOTATED{"nsDNSServer.mLock"};
+  mozilla::Mutex mLock{"nsDNSServer.mLock"};
 
   // mIPv4OnlyDomains is a comma-separated list of domains for which only
   // IPv4 DNS lookups are performed. This allows the user to disable IPv6 on
   // a per-domain basis and work around broken DNS servers. See bug 68796.
-  nsCString mIPv4OnlyDomains;
+  nsCString mIPv4OnlyDomains MOZ_GUARDED_BY(mLock);
   nsCString mForceResolve;
-  nsCString mMockHTTPSRRDomain;
+  nsCString mMockHTTPSRRDomain MOZ_GUARDED_BY(mLock);
   mozilla::Atomic<bool, mozilla::Relaxed> mHasMockHTTPSRRDomainSet{false};
   bool mNotifyResolution = false;
   bool mForceResolveOn = false;
-  nsTHashSet<nsCString> mLocalDomains;
+  nsTHashSet<nsCString> mLocalDomains MOZ_GUARDED_BY(mLock);
   RefPtr<mozilla::net::TRRService> mTrrService;
 
-  nsClassHashtable<nsCStringHashKey, nsTArray<nsCString>> mFailedSVCDomainNames;
+  nsClassHashtable<nsCStringHashKey, nsTArray<nsCString>> mFailedSVCDomainNames
+      MOZ_GUARDED_BY(mLock);
+  ;
 };
 
 already_AddRefed<nsIDNSService> GetOrInitDNSService();