Bug 1228640 - Backout bug 1183781 (changeset b9b6a1567ef6) for causing crash a=me
authorValentin Gosu <valentin.gosu@gmail.com>
Sat, 28 Nov 2015 00:20:01 +0100
changeset 308680 a6237ee1e852dd8c964765e68b7a5675ab113ca4
parent 308679 ac08219f171c4581bd422e5125e4ba393ede6308
child 308681 a16c2903506aa71c07f0ba3dda932bed03985d0c
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1228640, 1183781
milestone45.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 1228640 - Backout bug 1183781 (changeset b9b6a1567ef6) for causing crash a=me
build/sanitizers/lsan_suppressions.txt
netwerk/dns/DNS.cpp
netwerk/dns/DNS.h
netwerk/dns/GetAddrInfo.cpp
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- a/build/sanitizers/lsan_suppressions.txt
+++ b/build/sanitizers/lsan_suppressions.txt
@@ -84,16 +84,20 @@ leak:processInternalEntity
 leak:nss_ClearErrorStack
 
 # Bug 1122045 - Leaks in MessageLoop::MessageLoop()
 leak:MessageLoop::MessageLoop
 # This may not actually be related to 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
 
 
 ###
 ### 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
@@ -254,18 +254,16 @@ NetAddrElement::NetAddrElement(const Net
 {
   mAddress = netAddr.mAddress;
 }
 
 NetAddrElement::~NetAddrElement()
 {
 }
 
-NS_IMPL_ISUPPORTS0(AddrInfo)
-
 AddrInfo::AddrInfo(const char *host, const PRAddrInfo *prAddrInfo,
                    bool disableIPv4, bool filterNameCollision, const char *cname)
 {
   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;
--- 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 "nsISupports.h"
 
 #if !defined(XP_WIN)
 #include <arpa/inet.h>
 #endif
 
 #ifdef XP_WIN
 #include "winsock2.h"
 #endif
@@ -121,43 +120,40 @@ class NetAddrElement : public LinkedList
 public:
   explicit NetAddrElement(const PRNetAddr *prNetAddr);
   NetAddrElement(const NetAddrElement& netAddr);
   ~NetAddrElement();
 
   NetAddr mAddress;
 };
 
-class AddrInfo final
-  : public nsISupports {
+class AddrInfo {
 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,
            bool filterNameCollision, const char *cname);
 
   // 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;
 
   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);
-  ~AddrInfo();
 };
 
 // Copies the contents of a PRNetAddr to a NetAddr.
 // Does not do a ptr safety check!
 void PRNetAddrToNetAddr(const PRNetAddr *prAddr, NetAddr *addr);
 
 // Copies the contents of a NetAddr to a PRNetAddr.
 // Does not do a ptr safety check!
--- a/netwerk/dns/GetAddrInfo.cpp
+++ b/netwerk/dns/GetAddrInfo.cpp
@@ -357,24 +357,24 @@ static MOZ_ALWAYS_INLINE nsresult
   }
 
   const char* canonName = nullptr;
   if (aFlags & nsHostResolver::RES_CANON_NAME) {
     canonName = PR_GetCanonNameFromAddrInfo(prai);
   }
 
   bool filterNameCollision = !(aFlags & nsHostResolver::RES_ALLOW_NAME_COLLISION);
-  RefPtr<AddrInfo> ai(new AddrInfo(aCanonHost, prai, disableIPv4,
-                                   filterNameCollision, canonName));
+  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;
 }
 
 //////////////////////////////////////
 // COMMON/PLATFORM INDEPENDENT CODE //
 //////////////////////////////////////
 nsresult
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -219,16 +219,17 @@ 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",
@@ -1231,26 +1232,29 @@ 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);
+            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;
         }
 
         if (!mShutdown) {
             // add to mEvictionQ
             PR_APPEND_LINK(rec, &mEvictionQ);
@@ -1375,37 +1379,36 @@ nsHostResolver::ThreadFunc(void *arg)
     LOG(("DNS lookup thread - starting execution.\n"));
 
     static nsThreadPoolNaming naming;
     naming.SetThreadPoolName(NS_LITERAL_CSTRING("DNS Resolver"));
 
 #if defined(RES_RETRY_ON_FAILURE)
     nsResState rs;
 #endif
-    RefPtr<nsHostResolver> resolver = dont_AddRef(static_cast<nsHostResolver*>(arg));
+    nsHostResolver *resolver = (nsHostResolver *)arg;
     nsHostRecord *rec  = nullptr;
-    RefPtr<AddrInfo> ai;
+    AddrInfo *ai = nullptr;
 
     while (rec || resolver->GetHostToLookup(&rec)) {
         LOG(("DNS lookup thread - Calling getaddrinfo for host [%s%s%s].\n",
              LOG_HOST(rec->host, rec->netInterface)));
 
         TimeStamp startTime = TimeStamp::Now();
 #if TTL_AVAILABLE
         bool getTtl = rec->mGetTtl;
 #else
         bool getTtl = false;
 #endif
 
         nsresult status = GetAddrInfo(rec->host, rec->af, rec->flags, rec->netInterface,
-                                      getter_AddRefs(ai), getTtl);
+                                      &ai, getTtl);
 #if defined(RES_RETRY_ON_FAILURE)
         if (NS_FAILED(status) && rs.Reset()) {
-            status = GetAddrInfo(rec->host, rec->af, rec->flags, rec->netInterface,
-                                 getter_AddRefs(ai),
+            status = GetAddrInfo(rec->host, rec->af, rec->flags, rec->netInterface, &ai,
                                  getTtl);
         }
 #endif
 
         {   // obtain lock to check shutdown and manage inter-module telemetry
             MutexAutoLock lock(resolver->mLock);
 
             if (!resolver->mShutdown) {
@@ -1440,34 +1443,35 @@ nsHostResolver::ThreadFunc(void *arg)
             // leave 'rec' assigned and loop to make a renewed host resolve
             LOG(("DNS lookup thread - Re-resolving host [%s%s%s].\n",
                  LOG_HOST(rec->host, rec->netInterface)));
         } else {
             rec = nullptr;
         }
     }
     resolver->mThreadCount--;
+    NS_RELEASE(resolver);
     LOG(("DNS lookup thread - queue empty, thread finished.\n"));
 }
 
 nsresult
 nsHostResolver::Create(uint32_t maxCacheEntries,
                        uint32_t defaultCacheEntryLifetime,
                        uint32_t defaultGracePeriod,
                        nsHostResolver **result)
 {
-    RefPtr<nsHostResolver> res =
-        new nsHostResolver(maxCacheEntries, defaultCacheEntryLifetime,
-                           defaultGracePeriod);
+    nsHostResolver *res = new nsHostResolver(maxCacheEntries, defaultCacheEntryLifetime,
+                                             defaultGracePeriod);
+    NS_ADDREF(res);
 
     nsresult rv = res->Init();
-    if (NS_SUCCEEDED(rv)) {
-        res.forget(result);
-    }
+    if (NS_FAILED(rv))
+        NS_RELEASE(res);
 
+    *result = res;
     return rv;
 }
 
 void
 nsHostResolver::GetDNSCacheEntries(nsTArray<DNSCacheEntries> *args)
 {
     for (auto iter = mDB.Iter(); !iter.Done(); iter.Next()) {
         // We don't pay attention to address literals, only resolved domains.
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -68,17 +68,17 @@ 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| */
-    RefPtr<mozilla::net::AddrInfo> addr_info;
+    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,