Bug 1301069 - Backed out bug 1185120 (changeset 38cad72a77a6) a=backout
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 08 Sep 2016 00:04:07 +0200
changeset 354445 9330dc01ec0245f05311c56025a50705138351bc
parent 354444 4d0d2eb4d0f4d656937c602f443e40aa48caebc1
child 354446 fdfb1d87051b55aafb9ff6e3ea8482a527402191
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1301069, 1185120
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1301069 - Backed out bug 1185120 (changeset 38cad72a77a6) a=backout
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,