Bug 1301069 - Backed out bug 1185120 (changeset 38cad72a77a6) a=backout a=merge
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 08 Sep 2016 00:04:07 +0200
changeset 313090 95c87737b9d769f9c0e5ff9dc03b5cb2e75fbc38
parent 313089 3f6286f7ce480c890cfe5a97149dfc86bff797b7
child 313091 77940cbf0c2a9f52c209fbbde5b2e7d4c74a1501
push id20479
push userkwierso@gmail.com
push dateThu, 08 Sep 2016 01:08:46 +0000
treeherderfx-team@fb7c6b034329 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout, merge
bugs1301069, 1185120
milestone51.0a1
Bug 1301069 - Backed out bug 1185120 (changeset 38cad72a77a6) a=backout a=merge
build/sanitizers/lsan_suppressions.txt
netwerk/dns/DNS.cpp
netwerk/dns/DNS.h
netwerk/dns/GetAddrInfo.cpp
netwerk/dns/nsDNSService2.cpp
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- a/build/sanitizers/lsan_suppressions.txt
+++ b/build/sanitizers/lsan_suppressions.txt
@@ -71,16 +71,20 @@ leak:nsPSPrinterList::GetPrinterList
 leak:_PR_Getfd
 
 # Bug 1028483 - The XML parser sometimes leaks an object. bc3
 leak:processInternalEntity
 
 # Bug 1187421 - With e10s, NSS does not always free the error stack. m1.
 leak:nss_ClearErrorStack
 
+# 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
 
 
 ###
 ### Leaks with system libraries in their stacks. These show up across a number of tests.
 ### Better symbols and disabling fast stackwalking may help diagnose these.
--- a/netwerk/dns/DNS.cpp
+++ b/netwerk/dns/DNS.cpp
@@ -279,17 +279,19 @@ NetAddrElement::NetAddrElement(const Net
 }
 
 NetAddrElement::~NetAddrElement()
 {
 }
 
 AddrInfo::AddrInfo(const char *host, const PRAddrInfo *prAddrInfo,
                    bool disableIPv4, bool filterNameCollision, const char *cname)
-  : ttl(NO_TTL_DATA)
+  : mHostName(nullptr)
+  , mCanonicalName(nullptr)
+  , ttl(NO_TTL_DATA)
 {
   MOZ_ASSERT(prAddrInfo, "Cannot construct AddrInfo with a null prAddrInfo pointer!");
   const uint32_t nameCollisionAddr = htonl(0x7f003535); // 127.0.53.53
 
   Init(host, cname);
   PRNetAddr tmpAddr;
   void *iter = nullptr;
   do {
@@ -300,58 +302,64 @@ AddrInfo::AddrInfo(const char *host, con
     if (addIt) {
         NetAddrElement *addrElement = new NetAddrElement(&tmpAddr);
         mAddresses.insertBack(addrElement);
     }
   } while (iter);
 }
 
 AddrInfo::AddrInfo(const char *host, const char *cname)
-  : ttl(NO_TTL_DATA)
+  : mHostName(nullptr)
+  , mCanonicalName(nullptr)
+  , ttl(NO_TTL_DATA)
 {
   Init(host, cname);
 }
 
 AddrInfo::~AddrInfo()
 {
   NetAddrElement *addrElement;
   while ((addrElement = mAddresses.popLast())) {
     delete addrElement;
   }
+  free(mHostName);
+  free(mCanonicalName);
 }
 
 void
 AddrInfo::Init(const char *host, const char *cname)
 {
   MOZ_ASSERT(host, "Cannot initialize AddrInfo with a null host pointer!");
 
   ttl = NO_TTL_DATA;
   size_t hostlen = strlen(host);
-  mHostName = mozilla::MakeUnique<char[]>(hostlen + 1);
-  memcpy(mHostName.get(), host, hostlen + 1);
-
+  mHostName = static_cast<char*>(moz_xmalloc(hostlen + 1));
+  memcpy(mHostName, host, hostlen + 1);
   if (cname) {
     size_t cnameLen = strlen(cname);
-    mCanonicalName = mozilla::MakeUnique<char[]>(cnameLen + 1);
-    memcpy(mCanonicalName.get(), cname, cnameLen + 1);
+    mCanonicalName = static_cast<char*>(moz_xmalloc(cnameLen + 1));
+    memcpy(mCanonicalName, cname, cnameLen + 1);
+  }
+  else {
+    mCanonicalName = nullptr;
   }
 }
 
 void
 AddrInfo::AddAddress(NetAddrElement *address)
 {
   MOZ_ASSERT(address, "Cannot add the address to an uninitialized list");
 
   mAddresses.insertBack(address);
 }
 
 size_t
 AddrInfo::SizeOfIncludingThis(MallocSizeOf mallocSizeOf) const
 {
   size_t n = mallocSizeOf(this);
-  n += mallocSizeOf(mHostName.get());
-  n += mallocSizeOf(mCanonicalName.get());
+  n += mallocSizeOf(mHostName);
+  n += mallocSizeOf(mCanonicalName);
   n += mAddresses.sizeOfExcludingThis(mallocSizeOf);
   return n;
 }
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/dns/DNS.h
+++ b/netwerk/dns/DNS.h
@@ -8,17 +8,16 @@
 #define DNS_h_
 
 #include "nscore.h"
 #include "prio.h"
 #include "prnetdb.h"
 #include "plstr.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/MemoryReporting.h"
-#include "mozilla/UniquePtr.h"
 
 #if !defined(XP_WIN)
 #include <arpa/inet.h>
 #endif
 
 #ifdef XP_WIN
 #include "winsock2.h"
 #endif
@@ -137,18 +136,18 @@ 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);
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
-  UniquePtr<char[]> mHostName;
-  UniquePtr<char[]> mCanonicalName;
+  char *mHostName;
+  char *mCanonicalName;
   uint16_t ttl;
   static const uint16_t NO_TTL_DATA = (uint16_t) -1;
 
   LinkedList<NetAddrElement> mAddresses;
 
 private:
   void Init(const char *host, const char *cname);
 };
--- a/netwerk/dns/GetAddrInfo.cpp
+++ b/netwerk/dns/GetAddrInfo.cpp
@@ -340,17 +340,17 @@ GetAddrInfo(const char* aHost, uint16_t 
                                       aNetworkInterface, aAddrInfo);
 
 #if DNSQUERY_AVAILABLE
   if (aGetTtl && NS_SUCCEEDED(rv)) {
     // Figure out the canonical name, or if that fails, just use the host name
     // we have.
     const char *name = nullptr;
     if (*aAddrInfo != nullptr && (*aAddrInfo)->mCanonicalName) {
-      name = (*aAddrInfo)->mCanonicalName.get();
+      name = (*aAddrInfo)->mCanonicalName;
     } else {
       name = aHost;
     }
 
     LOG("Getting TTL for %s (cname = %s).", aHost, name);
     uint16_t ttl = 0;
     nsresult ttlRv = _GetTTLData_Windows(name, &ttl, aAddressFamily);
     if (NS_SUCCEEDED(ttlRv)) {
--- a/netwerk/dns/nsDNSService2.cpp
+++ b/netwerk/dns/nsDNSService2.cpp
@@ -88,19 +88,19 @@ nsDNSRecord::GetCanonicalName(nsACString
                    NS_ERROR_NOT_AVAILABLE);
 
     // if the record is for an IP address literal, then the canonical
     // host name is the IP address literal.
     const char *cname;
     {
         MutexAutoLock lock(mHostRecord->addr_info_lock);
         if (mHostRecord->addr_info)
-            cname = !mHostRecord->addr_info->mCanonicalName.get() ?
-                mHostRecord->addr_info->mCanonicalName.get() :
-                mHostRecord->addr_info->mHostName.get();
+            cname = mHostRecord->addr_info->mCanonicalName ?
+                mHostRecord->addr_info->mCanonicalName :
+                mHostRecord->addr_info->mHostName;
         else
             cname = mHostRecord->host;
         result.Assign(cname);
     }
     return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -151,17 +151,17 @@ nsDNSRecord::GetNextAddr(uint16_t port, 
         mHostRecord->addr_info_lock.Unlock();
 
         if (!mHostRecord->addr) {
             // Both mHostRecord->addr_info and mHostRecord->addr are null.
             // This can happen if mHostRecord->addr_info expired and the
             // attempt to reresolve it failed.
             return NS_ERROR_NOT_AVAILABLE;
         }
-        memcpy(addr, mHostRecord->addr.get(), sizeof(NetAddr));
+        memcpy(addr, mHostRecord->addr, sizeof(NetAddr));
         mDone = true;
     }
 
     // set given port
     port = htons(port);
     if (addr->raw.family == AF_INET) {
         addr->inet.port = port;
     } else if (addr->raw.family == AF_INET6) {
@@ -196,17 +196,17 @@ nsDNSRecord::GetAddresses(nsTArray<NetAd
         mHostRecord->addr_info_lock.Unlock();
     } else {
         mHostRecord->addr_info_lock.Unlock();
 
         if (!mHostRecord->addr) {
             return NS_ERROR_NOT_AVAILABLE;
         }
         NetAddr *addr = aAddressArray.AppendElement(NetAddr());
-        memcpy(addr, mHostRecord->addr.get(), sizeof(NetAddr));
+        memcpy(addr, mHostRecord->addr, sizeof(NetAddr));
         if (addr->raw.family == AF_INET) {
             addr->inet.port = 0;
         } else if (addr->raw.family == AF_INET6) {
             addr->inet6.port = 0;
         }
     }
     return NS_OK;
 }
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -159,16 +159,18 @@ IsLowPriority(uint16_t flags)
 // this macro filters out any flags that are not used when constructing the
 // host key.  the significant flags are those that would affect the resulting
 // host record (i.e., the flags that are passed down to PR_GetAddrInfoByName).
 #define RES_KEY_FLAGS(_f) ((_f) & nsHostResolver::RES_CANON_NAME)
 
 nsHostRecord::nsHostRecord(const nsHostKey *key)
     : addr_info_lock("nsHostRecord.addr_info_lock")
     , addr_info_gencnt(0)
+    , addr_info(nullptr)
+    , addr(nullptr)
     , negative(false)
     , resolving(false)
     , onQueue(false)
     , usingAnyThread(false)
     , mDoomed(false)
 #if TTL_AVAILABLE
     , mGetTtl(false)
 #endif
@@ -219,16 +221,18 @@ nsHostRecord::CopyExpirationTimesAndFlag
     mValidEnd = aFromHostRecord->mValidEnd;
     mGraceStart = aFromHostRecord->mGraceStart;
     mDoomed = aFromHostRecord->mDoomed;
 }
 
 nsHostRecord::~nsHostRecord()
 {
     Telemetry::Accumulate(Telemetry::DNS_BLACKLIST_COUNT, mBlacklistedCount);
+    delete addr_info;
+    delete addr;
 }
 
 bool
 nsHostRecord::Blacklisted(NetAddr *aQuery)
 {
     // must call locked
     LOG(("Checking blacklist for host [%s%s%s], host record [%p].\n",
           LOG_HOST(host, netInterface), this));
@@ -308,17 +312,17 @@ nsHostRecord::HasUsableResult(const mozi
     if (negative && IsHighPriority(queryFlags)) {
         return false;
     }
 
     if (CheckExpiration(now) == EXP_EXPIRED) {
         return false;
     }
 
-    return addr_info.get() || addr.get() || negative;
+    return addr_info || addr || negative;
 }
 
 static size_t
 SizeOfResolveHostCallbackListExcludingHead(const PRCList *head,
                                            MallocSizeOf mallocSizeOf)
 {
     size_t n = 0;
     PRCList *curr = head->next;
@@ -338,17 +342,17 @@ nsHostRecord::SizeOfIncludingThis(Malloc
 
     // The |host| field (inherited from nsHostKey) actually points to extra
     // memory that is allocated beyond the end of the nsHostRecord (see
     // nsHostRecord::Create()).  So it will be included in the
     // |mallocSizeOf(this)| call above.
 
     n += SizeOfResolveHostCallbackListExcludingHead(&callbacks, mallocSizeOf);
     n += addr_info ? addr_info->SizeOfIncludingThis(mallocSizeOf) : 0;
-    n += mallocSizeOf(addr.get());
+    n += mallocSizeOf(addr);
 
     n += mBlacklistedItems.ShallowSizeOfExcludingThis(mallocSizeOf);
     for (size_t i = 0; i < mBlacklistedItems.Length(); i++) {
         n += mBlacklistedItems[i].SizeOfExcludingThisIfUnshared(mallocSizeOf);
     }
     return n;
 }
 
@@ -798,18 +802,18 @@ nsHostResolver::ResolveHost(const char  
             }
             // try parsing the host name as an IP address literal to short
             // circuit full host resolution.  (this is necessary on some
             // platforms like Win9x.  see bug 219376 for more details.)
             else if (PR_StringToNetAddr(host, &tempAddr) == PR_SUCCESS) {
                 LOG(("  Host is IP Literal [%s].\n", host));
                 // ok, just copy the result into the host record, and be done
                 // with it! ;-)
-                he->rec->addr.reset(new NetAddr());
-                PRNetAddrToNetAddr(&tempAddr, he->rec->addr.get());
+                he->rec->addr = new NetAddr();
+                PRNetAddrToNetAddr(&tempAddr, he->rec->addr);
                 // put reference to host record on stack...
                 Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
                                       METHOD_LITERAL);
                 result = he->rec;
             }
             else if (mPendingCount >= MAX_NON_PRIORITY_REQUESTS &&
                      !IsHighPriority(flags) &&
                      !he->rec->resolving) {
@@ -861,19 +865,19 @@ nsHostResolver::ResolveHost(const char  
                             // in the cache (not blacklisted and from the right
                             // family).
                             NetAddrElement *addrIter =
                                 unspecHe->rec->addr_info->mAddresses.getFirst();
                             while (addrIter) {
                                 if ((af == addrIter->mAddress.inet.family) &&
                                      !unspecHe->rec->Blacklisted(&addrIter->mAddress)) {
                                     if (!he->rec->addr_info) {
-                                        he->rec->addr_info.reset(new AddrInfo(
-                                            unspecHe->rec->addr_info->mHostName.get(),
-                                            unspecHe->rec->addr_info->mCanonicalName.get()));
+                                        he->rec->addr_info = new AddrInfo(
+                                            unspecHe->rec->addr_info->mHostName,
+                                            unspecHe->rec->addr_info->mCanonicalName);
                                         he->rec->CopyExpirationTimesAndFlagsFrom(unspecHe->rec);
                                     }
                                     he->rec->addr_info->AddAddress(
                                         new NetAddrElement(*addrIter));
                                 }
                                 addrIter = addrIter->getNext();
                             }
                         }
@@ -1213,17 +1217,17 @@ nsHostResolver::PrepareRecordExpiration(
 
 static bool
 different_rrset(AddrInfo *rrset1, AddrInfo *rrset2)
 {
     if (!rrset1 || !rrset2) {
         return true;
     }
 
-    LOG(("different_rrset %s\n", rrset1->mHostName.get()));
+    LOG(("different_rrset %s\n", rrset1->mHostName));
     nsTArray<NetAddr> orderedSet1;
     nsTArray<NetAddr> orderedSet2;
 
     for (NetAddrElement *element = rrset1->mAddresses.getFirst();
          element; element = element->getNext()) {
         if (LOG_ENABLED()) {
             char buf[128];
             NetAddrToString(&element->mAddress, buf, 128);
@@ -1280,34 +1284,37 @@ nsHostResolver::OnLookupComplete(nsHostR
             return LOOKUP_RESOLVEAGAIN;
         }
 
         // grab list of callbacks to notify
         MoveCList(rec->callbacks, cbs);
 
         // 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);
-            if (different_rrset(rec->addr_info.get(), newRRSet)) {
+            if (different_rrset(rec->addr_info, newRRSet)) {
                 LOG(("nsHostResolver record %p new gencnt\n", rec));
-                rec->addr_info.reset(newRRSet);
+                old_addr_info = rec->addr_info;
+                rec->addr_info = newRRSet;
                 rec->addr_info_gencnt++;
             } else {
                 if (rec->addr_info && newRRSet) {
                     rec->addr_info->ttl = newRRSet->ttl;
                 }
-                delete newRRSet;
+                old_addr_info = newRRSet;
             }
         }
+        delete old_addr_info;
 
         rec->negative = !rec->addr_info;
         PrepareRecordExpiration(rec);
         rec->resolving = false;
-
+        
         if (rec->usingAnyThread) {
             mActiveAnyThreadCount--;
             rec->usingAnyThread = false;
         }
 
         if (!mShutdown) {
             // add to mEvictionQ
             PR_APPEND_LINK(rec, &mEvictionQ);
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -16,17 +16,16 @@
 #include "nsIDNSListener.h"
 #include "nsIDNSService.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "GetAddrInfo.h"
 #include "mozilla/net/DNS.h"
 #include "mozilla/net/DashboardTypes.h"
 #include "mozilla/TimeStamp.h"
-#include "mozilla/UniquePtr.h"
 
 class nsHostResolver;
 class nsHostRecord;
 class nsResolveHostCallback;
 
 #define MAX_RESOLVER_THREADS_FOR_ANY_PRIORITY  3
 #define MAX_RESOLVER_THREADS_FOR_HIGH_PRIORITY 5
 #define MAX_NON_PRIORITY_REQUESTS 150
@@ -69,18 +68,18 @@ public:
      * nsDNSService2 class.  |addr| doesn't change after it has been
      * assigned a value.  only the resolver worker thread modifies
      * nsHostRecord (and only in nsHostResolver::OnLookupComplete);
      * the other threads just read it.  therefore the resolver worker
      * thread doesn't need to lock when reading |addr_info|.
      */
     Mutex        addr_info_lock;
     int          addr_info_gencnt; /* generation count of |addr_info| */
-    mozilla::UniquePtr<mozilla::net::AddrInfo> addr_info;
-    mozilla::UniquePtr<mozilla::net::NetAddr>  addr;
+    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
                                 (though never for more than 60 seconds), but a use
                                 of that negative entry forces an asynchronous refresh. */
 
     enum ExpirationStatus {
         EXP_VALID,
         EXP_GRACE,