Bug 1183781 - Small leak in DNS.cpp and nsHostResolver.cpp r=sworkman

* makes AddrInfo extend nsISupports
* uses RefPtr for managing AddrInfo and nsHostResolver in nsHostResolver.cpp

build/sanitizers/lsan_suppressions.txt

@@ -89,10 +89,6 @@ leak:MessageLoop::MessageLoop
 leak:base::WaitableEvent::TimedWait
 leak:MessageLoop::PostTask_Helper
 
-# Bug 1189430 - DNS leaks in mochitest-chrome.
-leak:nsDNSService::AsyncResolveExtended
-leak:_GetAddrInfo_Portable
-
 # Bug 1189568 - Indirect leaks of IMContextWrapper and nsIntRect.
 leak:nsWindow::Create
 leak:nsBaseWidget::StoreWindowClipRegion

netwerk/dns/DNS.cpp

@@ -259,6 +259,8 @@ NetAddrElement::~NetAddrElement()
 {
 }
 
+NS_IMPL_ISUPPORTS0(AddrInfo)
+
 AddrInfo::AddrInfo(const char *host, const PRAddrInfo *prAddrInfo,
                    bool disableIPv4, bool filterNameCollision, const char *cname)
 {

netwerk/dns/DNS.h

@@ -13,6 +13,7 @@
 #include "plstr.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/MemoryReporting.h"
+#include "nsISupports.h"
 
 #if !defined(XP_WIN)
 #include <arpa/inet.h>
@@ -125,8 +126,11 @@ public:
   NetAddr mAddress;
 };
 
-class AddrInfo {
+class AddrInfo final
+  : public nsISupports {
 public:
+  NS_DECL_THREADSAFE_ISUPPORTS
+
   // Creates an AddrInfo object. It calls the AddrInfo(const char*, const char*)
   // to initialize the host and the cname.
   AddrInfo(const char *host, const PRAddrInfo *prAddrInfo, bool disableIPv4,
@@ -134,7 +138,6 @@ public:
 
   // Creates a basic AddrInfo object (initialize only the host and the cname).
   AddrInfo(const char *host, const char *cname);
-  ~AddrInfo();
 
   void AddAddress(NetAddrElement *address);
 
@@ -149,6 +152,7 @@ public:
 
 private:
   void Init(const char *host, const char *cname);
+  ~AddrInfo();
 };
 
 // Copies the contents of a PRNetAddr to a NetAddr.

netwerk/dns/GetAddrInfo.cpp

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

netwerk/dns/nsHostResolver.cpp

@@ -224,7 +224,6 @@ nsHostRecord::CopyExpirationTimesAndFlagsFrom(const nsHostRecord *aFromHostRecor
 nsHostRecord::~nsHostRecord()
 {
     Telemetry::Accumulate(Telemetry::DNS_BLACKLIST_COUNT, mBlacklistedCount);
-    delete addr_info;
     delete addr;
 }
 
@@ -1237,19 +1236,16 @@ nsHostResolver::OnLookupComplete(nsHostRecord* rec, nsresult status, AddrInfo* r
 
         // update record fields.  We might have a rec->addr_info already if a
         // previous lookup result expired and we're reresolving it..
-        AddrInfo  *old_addr_info;
         {
             MutexAutoLock lock(rec->addr_info_lock);
-            old_addr_info = rec->addr_info;
             rec->addr_info = result;
             rec->addr_info_gencnt++;
         }
-        delete old_addr_info;
 
         rec->negative = !rec->addr_info;
         PrepareRecordExpiration(rec);
         rec->resolving = false;
-        
+
         if (rec->usingAnyThread) {
             mActiveAnyThreadCount--;
             rec->usingAnyThread = false;
@@ -1384,9 +1380,9 @@ nsHostResolver::ThreadFunc(void *arg)
 #if defined(RES_RETRY_ON_FAILURE)
     nsResState rs;
 #endif
-    nsHostResolver *resolver = (nsHostResolver *)arg;
+    RefPtr<nsHostResolver> resolver = dont_AddRef(static_cast<nsHostResolver*>(arg));
     nsHostRecord *rec  = nullptr;
-    AddrInfo *ai = nullptr;
+    RefPtr<AddrInfo> ai;
 
     while (rec || resolver->GetHostToLookup(&rec)) {
         LOG(("DNS lookup thread - Calling getaddrinfo for host [%s%s%s].\n",
@@ -1400,10 +1396,11 @@ nsHostResolver::ThreadFunc(void *arg)
 #endif
 
         nsresult status = GetAddrInfo(rec->host, rec->af, rec->flags, rec->netInterface,
-                                      &ai, getTtl);
+                                      getter_AddRefs(ai), getTtl);
 #if defined(RES_RETRY_ON_FAILURE)
         if (NS_FAILED(status) && rs.Reset()) {
-            status = GetAddrInfo(rec->host, rec->af, rec->flags, rec->netInterface, &ai,
+            status = GetAddrInfo(rec->host, rec->af, rec->flags, rec->netInterface,
+                                 getter_AddRefs(ai),
                                  getTtl);
         }
 #endif
@@ -1448,7 +1445,6 @@ nsHostResolver::ThreadFunc(void *arg)
         }
     }
     resolver->mThreadCount--;
-    NS_RELEASE(resolver);
     LOG(("DNS lookup thread - queue empty, thread finished.\n"));
 }
 
@@ -1458,15 +1454,15 @@ nsHostResolver::Create(uint32_t maxCacheEntries,
                        uint32_t defaultGracePeriod,
                        nsHostResolver **result)
 {
-    nsHostResolver *res = new nsHostResolver(maxCacheEntries, defaultCacheEntryLifetime,
-                                             defaultGracePeriod);
-    NS_ADDREF(res);
+    RefPtr<nsHostResolver> res =
+        new nsHostResolver(maxCacheEntries, defaultCacheEntryLifetime,
+                           defaultGracePeriod);
 
     nsresult rv = res->Init();
-    if (NS_FAILED(rv))
-        NS_RELEASE(res);
+    if (NS_SUCCEEDED(rv)) {
+        res.forget(result);
+    }
 
-    *result = res;
     return rv;
 }
 

netwerk/dns/nsHostResolver.h

@@ -73,7 +73,7 @@ public:
      */
     Mutex        addr_info_lock;
     int          addr_info_gencnt; /* generation count of |addr_info| */
-    mozilla::net::AddrInfo *addr_info;
+    RefPtr<mozilla::net::AddrInfo> addr_info;
     mozilla::net::NetAddr  *addr;
     bool         negative;   /* True if this record is a cache of a failed lookup.
                                 Negative cache entries are valid just like any other