Backed out 2 changesets (bug 1420677) for causing bug 1534550 a=backout

Backed out changeset cf114035c79f (bug 1420677)
Backed out changeset edff1f39d426 (bug 1420677)

netwerk/dns/DNS.cpp

@@ -330,6 +330,10 @@ AddrInfo::AddrInfo(const AddrInfo *src) {
 }
 
 AddrInfo::~AddrInfo() {
+  NetAddrElement *addrElement;
+  while ((addrElement = mAddresses.popLast())) {
+    delete addrElement;
+  }
 }
 
 void AddrInfo::AddAddress(NetAddrElement *address) {

netwerk/dns/DNS.h

@@ -12,7 +12,6 @@
 #include "prio.h"
 #include "prnetdb.h"
 #include "plstr.h"
-#include "nsISupportsImpl.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/MemoryReporting.h"
 
@@ -124,8 +123,6 @@ class NetAddrElement : public LinkedListElement<NetAddrElement> {
 };
 
 class AddrInfo {
-  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(AddrInfo)
-
  public:
   // Creates an AddrInfo object.
   explicit AddrInfo(const nsACString &host, const PRAddrInfo *prAddrInfo,
@@ -139,6 +136,7 @@ class AddrInfo {
 
   // Creates a basic AddrInfo object (initialize only the host and TRR status).
   explicit AddrInfo(const nsACString &host, unsigned int TRRType);
+  ~AddrInfo();
 
   explicit AddrInfo(const AddrInfo *src);  // copy
 
@@ -151,11 +149,10 @@ class AddrInfo {
   uint32_t ttl;
   static const uint32_t NO_TTL_DATA = (uint32_t)-1;
 
-  AutoCleanLinkedList<NetAddrElement> mAddresses;
+  LinkedList<NetAddrElement> mAddresses;
   unsigned int IsTRR() { return mFromTRR; }
 
  private:
-  ~AddrInfo();
   unsigned int mFromTRR;
 };
 

netwerk/dns/GetAddrInfo.cpp

@@ -277,14 +277,14 @@ _GetAddrInfo_Portable(const nsACString& aCanonHost, uint16_t aAddressFamily,
 
   bool filterNameCollision =
       !(aFlags & nsHostResolver::RES_ALLOW_NAME_COLLISION);
-  RefPtr<AddrInfo> ai(new AddrInfo(aCanonHost, prai, disableIPv4,
+  nsAutoPtr<AddrInfo> ai(new AddrInfo(aCanonHost, prai, disableIPv4,
                                       filterNameCollision, canonName));
   PR_FreeAddrInfo(prai);
   if (ai->mAddresses.isEmpty()) {
     return NS_ERROR_UNKNOWN_HOST;
   }
 
-  ai.forget(aAddrInfo);
+  *aAddrInfo = ai.forget();
 
   return NS_OK;
 }

netwerk/dns/TRR.cpp

@@ -876,7 +876,7 @@ nsresult TRR::DohDecode(nsCString &aHost) {
 nsresult TRR::ReturnData() {
   if (mType != TRRTYPE_TXT) {
     // create and populate an AddrInfo instance to pass on
-    RefPtr<AddrInfo> ai(new AddrInfo(mHost, mType));
+    nsAutoPtr<AddrInfo> ai(new AddrInfo(mHost, mType));
     DOHaddr *item;
     uint32_t ttl = AddrInfo::NO_TTL_DATA;
     while ((item = static_cast<DOHaddr *>(mDNS.mAddresses.popFirst()))) {
@@ -895,7 +895,7 @@ nsresult TRR::ReturnData() {
     if (!mHostResolver) {
       return NS_ERROR_FAILURE;
     }
-    (void)mHostResolver->CompleteLookup(mRec, NS_OK, ai, mPB,
+    (void)mHostResolver->CompleteLookup(mRec, NS_OK, ai.forget(), mPB,
                                         mOriginSuffix);
     mHostResolver = nullptr;
     mRec = nullptr;
@@ -915,7 +915,7 @@ nsresult TRR::FailData(nsresult error) {
   } else {
     // create and populate an TRR AddrInfo instance to pass on to signal that
     // this comes from TRR
-    RefPtr<AddrInfo> ai = new AddrInfo(mHost, mType);
+    AddrInfo *ai = new AddrInfo(mHost, mType);
 
     (void)mHostResolver->CompleteLookup(mRec, error, ai, mPB, mOriginSuffix);
   }

netwerk/dns/TRRService.cpp

@@ -612,7 +612,7 @@ AHostResolver::LookupStatus TRRService::CompleteLookup(
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!rec);
 
-  RefPtr<AddrInfo> newRRSet(aNewRRSet);
+  nsAutoPtr<AddrInfo> newRRSet(aNewRRSet);
   MOZ_ASSERT(newRRSet && newRRSet->IsTRR() == TRRTYPE_NS);
 
 #ifdef DEBUG

netwerk/dns/nsHostResolver.cpp

@@ -297,6 +297,7 @@ AddrHostRecord::AddrHostRecord(const nsHostKey &key)
 AddrHostRecord::~AddrHostRecord() {
   mCallbacks.clear();
   Telemetry::Accumulate(Telemetry::DNS_BLACKLIST_COUNT, mBlacklistedCount);
+  delete addr_info;
 }
 
 bool AddrHostRecord::Blacklisted(NetAddr *aQuery) {
@@ -1008,6 +1009,9 @@ nsresult nsHostResolver::ResolveHost(const nsACString &aHost, uint16_t type,
             // addr_info.
             MutexAutoLock lock(addrRec->addr_info_lock);
 
+            // XXX: note that this actually leaks addr_info.
+            // For some reason, freeing the memory causes a crash in
+            // nsDNSRecord::GetNextAddr - see bug 1422173
             addrRec->addr_info = nullptr;
             if (unspecRec->negative) {
               rec->negative = unspecRec->negative;
@@ -1687,7 +1691,9 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup(
   RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
   MOZ_ASSERT(addrRec);
 
-  RefPtr<AddrInfo> newRRSet(aNewRRSet);
+  // newRRSet needs to be taken into the hostrecord (which will then own it)
+  // or deleted on early return.
+  nsAutoPtr<AddrInfo> newRRSet(aNewRRSet);
 
   bool trrResult = newRRSet && newRRSet->IsTRR();
 
@@ -1742,7 +1748,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup(
       // There's another TRR complete pending. Wait for it and keep
       // this RRset around until then.
       MOZ_ASSERT(!addrRec->mFirstTRR && newRRSet);
-      addrRec->mFirstTRR.swap(newRRSet);  // autoPtr.swap()
+      addrRec->mFirstTRR = newRRSet;  // autoPtr.swap()
       MOZ_ASSERT(addrRec->mFirstTRR && !newRRSet);
 
       if (addrRec->mDidCallbacks || addrRec->mResolverMode == MODE_SHADOW) {
@@ -1768,7 +1774,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup(
         if (NS_SUCCEEDED(status)) {
           merge_rrset(newRRSet, addrRec->mFirstTRR);
         } else {
-          newRRSet.swap(addrRec->mFirstTRR);  // transfers
+          newRRSet = addrRec->mFirstTRR;  // transfers
         }
         addrRec->mFirstTRR = nullptr;
       }
@@ -1817,7 +1823,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup(
   // note that we don't update the addr_info if this is trr shadow results
   if (!mShutdown && !(trrResult && addrRec->mResolverMode == MODE_SHADOW)) {
     MutexAutoLock lock(addrRec->addr_info_lock);
-    RefPtr<AddrInfo> old_addr_info;
+    nsAutoPtr<AddrInfo> old_addr_info;
     if (different_rrset(addrRec->addr_info, newRRSet)) {
       LOG(("nsHostResolver record %p new gencnt\n", addrRec.get()));
       old_addr_info = addrRec->addr_info;
@@ -2032,7 +2038,7 @@ void nsHostResolver::ThreadFunc() {
   nsResState rs;
 #endif
   RefPtr<AddrHostRecord> rec;
-  RefPtr<AddrInfo> ai;
+  AddrInfo *ai = nullptr;
 
   do {
     if (!rec) {
@@ -2053,10 +2059,10 @@ void nsHostResolver::ThreadFunc() {
     TimeDuration inQueue = startTime - rec->mNativeStart;
     uint32_t ms = static_cast<uint32_t>(inQueue.ToMilliseconds());
     Telemetry::Accumulate(Telemetry::DNS_NATIVE_QUEUING, ms);
-    nsresult status = GetAddrInfo(rec->host, rec->af, rec->flags, getter_AddRefs(ai), getTtl);
+    nsresult status = GetAddrInfo(rec->host, rec->af, rec->flags, &ai, getTtl);
 #if defined(RES_RETRY_ON_FAILURE)
     if (NS_FAILED(status) && rs.Reset()) {
-      status = GetAddrInfo(rec->host, rec->af, rec->flags, getter_AddRefs(ai), getTtl);
+      status = GetAddrInfo(rec->host, rec->af, rec->flags, &ai, getTtl);
     }
 #endif
 

netwerk/dns/nsHostResolver.h

@@ -181,7 +181,7 @@ class AddrHostRecord final : public nsHostRecord {
    */
   Mutex addr_info_lock;
   int addr_info_gencnt; /* generation count of |addr_info| */
-  RefPtr<mozilla::net::AddrInfo> addr_info;
+  mozilla::net::AddrInfo *addr_info;
   mozilla::UniquePtr<mozilla::net::NetAddr> addr;
 
   // hold addr_info_lock when calling the blacklist functions
@@ -222,7 +222,7 @@ class AddrHostRecord final : public nsHostRecord {
   mozilla::TimeDuration mTrrDuration;
   mozilla::TimeDuration mNativeDuration;
 
-  RefPtr<mozilla::net::AddrInfo> mFirstTRR;  // partial TRR storage
+  nsAutoPtr<mozilla::net::AddrInfo> mFirstTRR;  // partial TRR storage
   nsresult mFirstTRRresult;
 
   uint8_t mTRRSuccess;     // number of successful TRR responses