Bug 1196237 - Relemetry after dns shutdown. r=dragana, a=dveditz
authorPatrick McManus <mcmanus@ducksong.com>
Thu, 20 Aug 2015 12:14:40 -0400
changeset 288854 197a0c3b8d8e2a22fdfbcf1ace0e12f86bc5d51b
parent 288853 f973a86d54b17c2b11aced5489542fa0c20c0511
child 288855 20a79fcdc75fb0911b7a2b4ec93b5b11bca6d49b
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, dveditz
bugs1196237
milestone42.0a2
Bug 1196237 - Relemetry after dns shutdown. r=dragana, a=dveditz
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -517,23 +517,23 @@ static void DnsPrefChanged(const char* a
 nsHostResolver::nsHostResolver(uint32_t maxCacheEntries,
                                uint32_t defaultCacheEntryLifetime,
                                uint32_t defaultGracePeriod)
     : mMaxCacheEntries(maxCacheEntries)
     , mDefaultCacheLifetime(defaultCacheEntryLifetime)
     , mDefaultGracePeriod(defaultGracePeriod)
     , mLock("nsHostResolver.mLock")
     , mIdleThreadCV(mLock, "nsHostResolver.mIdleThreadCV")
+    , mDB(&gHostDB_ops, sizeof(nsHostDBEnt), 0)
+    , mEvictionQSize(0)
+    , mShutdown(true)
     , mNumIdleThreads(0)
     , mThreadCount(0)
     , mActiveAnyThreadCount(0)
-    , mDB(&gHostDB_ops, sizeof(nsHostDBEnt), 0)
-    , mEvictionQSize(0)
     , mPendingCount(0)
-    , mShutdown(true)
 {
     mCreationTime = PR_Now();
     PR_INIT_CLIST(&mHighQ);
     PR_INIT_CLIST(&mMediumQ);
     PR_INIT_CLIST(&mLowQ);
     PR_INIT_CLIST(&mEvictionQ);
 
     mLongIdleTimeout  = PR_SecondsToInterval(LongIdleTimeoutSeconds);
@@ -1065,20 +1065,20 @@ nsHostResolver::IssueLookup(nsHostRecord
     mPendingCount++;
     
     rec->resolving = true;
     rec->onQueue = true;
 
     rv = ConditionallyCreateThread(rec);
     
     LOG (("  DNS thread counters: total=%d any-live=%d idle=%d pending=%d\n",
-          mThreadCount,
-          mActiveAnyThreadCount,
-          mNumIdleThreads,
-          mPendingCount));
+          static_cast<uint32_t>(mThreadCount),
+          static_cast<uint32_t>(mActiveAnyThreadCount),
+          static_cast<uint32_t>(mNumIdleThreads),
+          static_cast<uint32_t>(mPendingCount)));
 
     return rv;
 }
 
 nsresult
 nsHostResolver::ConditionallyRefreshRecord(nsHostRecord *rec, const char *host)
 {
     if ((rec->CheckExpiration(TimeStamp::NowLoRes()) != nsHostRecord::EXP_VALID
@@ -1106,17 +1106,17 @@ nsHostResolver::DeQueue(PRCList &aQ, nsH
     (*aResult)->onQueue = false;
 }
 
 bool
 nsHostResolver::GetHostToLookup(nsHostRecord **result)
 {
     bool timedOut = false;
     PRIntervalTime epoch, now, timeout;
-    
+
     MutexAutoLock lock(mLock);
 
     timeout = (mNumIdleThreads >= HighThreadThreshold) ? mShortIdleTimeout : mLongIdleTimeout;
     epoch = PR_IntervalNow();
 
     while (!mShutdown) {
         // remove next record from Q; hand over owning reference. Check high, then med, then low
         
@@ -1174,17 +1174,16 @@ nsHostResolver::GetHostToLookup(nsHostRe
             // in which case we will loop back and re-enter it. In that case we want to
             // do so with the new timeout reduced to reflect time already spent waiting.
             timeout -= (PRIntervalTime)(now - epoch);
             epoch = now;
         }
     }
     
     // tell thread to exit...
-    mThreadCount--;
     return false;
 }
 
 void
 nsHostResolver::PrepareRecordExpiration(nsHostRecord* rec) const
 {
     MOZ_ASSERT(((bool)rec->addr_info) != rec->negative);
     if (!rec->addr_info) {
@@ -1406,49 +1405,57 @@ nsHostResolver::ThreadFunc(void *arg)
         nsresult status = GetAddrInfo(rec->host, rec->af, rec->flags, rec->netInterface,
                                       &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,
                                  getTtl);
         }
 #endif
-        TimeDuration elapsed = TimeStamp::Now() - startTime;
-        uint32_t millis = static_cast<uint32_t>(elapsed.ToMilliseconds());
+
+        {   // obtain lock to check shutdown and manage inter-module telemetry
+            MutexAutoLock lock(resolver->mLock);
+
+            if (!resolver->mShutdown) {
+                TimeDuration elapsed = TimeStamp::Now() - startTime;
+                uint32_t millis = static_cast<uint32_t>(elapsed.ToMilliseconds());
 
-        if (NS_SUCCEEDED(status)) {
-            Telemetry::ID histogramID;
-            if (!rec->addr_info_gencnt) {
-                // Time for initial lookup.
-                histogramID = Telemetry::DNS_LOOKUP_TIME;
-            } else if (!getTtl) {
-                // Time for renewal; categorized by expiration strategy.
-                histogramID = Telemetry::DNS_RENEWAL_TIME;
-            } else {
-                // Time to get TTL; categorized by expiration strategy.
-                histogramID = Telemetry::DNS_RENEWAL_TIME_FOR_TTL;
+                if (NS_SUCCEEDED(status)) {
+                    Telemetry::ID histogramID;
+                    if (!rec->addr_info_gencnt) {
+                        // Time for initial lookup.
+                        histogramID = Telemetry::DNS_LOOKUP_TIME;
+                    } else if (!getTtl) {
+                        // Time for renewal; categorized by expiration strategy.
+                        histogramID = Telemetry::DNS_RENEWAL_TIME;
+                    } else {
+                        // Time to get TTL; categorized by expiration strategy.
+                        histogramID = Telemetry::DNS_RENEWAL_TIME_FOR_TTL;
+                    }
+                    Telemetry::Accumulate(histogramID, millis);
+                } else {
+                    Telemetry::Accumulate(Telemetry::DNS_FAILED_LOOKUP_TIME, millis);
+                }
             }
-            Telemetry::Accumulate(histogramID, millis);
-        } else {
-            Telemetry::Accumulate(Telemetry::DNS_FAILED_LOOKUP_TIME, millis);
         }
 
         // OnLookupComplete may release "rec", long before we lose it.
         LOG(("DNS lookup thread - lookup completed for host [%s%s%s]: %s.\n",
              LOG_HOST(rec->host, rec->netInterface),
              ai ? "success" : "failure: unknown host"));
 
         if (LOOKUP_RESOLVEAGAIN == resolver->OnLookupComplete(rec, status, ai)) {
             // 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,
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -337,31 +337,32 @@ private:
         METHOD_NETWORK_SHARED = 7
     };
 
     uint32_t      mMaxCacheEntries;
     uint32_t      mDefaultCacheLifetime; // granularity seconds
     uint32_t      mDefaultGracePeriod; // granularity seconds
     mutable Mutex mLock;    // mutable so SizeOfIncludingThis can be const
     CondVar       mIdleThreadCV;
-    uint32_t      mNumIdleThreads;
-    uint32_t      mThreadCount;
-    uint32_t      mActiveAnyThreadCount;
     PLDHashTable  mDB;
     PRCList       mHighQ;
     PRCList       mMediumQ;
     PRCList       mLowQ;
     PRCList       mEvictionQ;
     uint32_t      mEvictionQSize;
-    uint32_t      mPendingCount;
     PRTime        mCreationTime;
-    bool          mShutdown;
     PRIntervalTime mLongIdleTimeout;
     PRIntervalTime mShortIdleTimeout;
 
+    mozilla::Atomic<bool>     mShutdown;
+    mozilla::Atomic<uint32_t> mNumIdleThreads;
+    mozilla::Atomic<uint32_t> mThreadCount;
+    mozilla::Atomic<uint32_t> mActiveAnyThreadCount;
+    mozilla::Atomic<uint32_t> mPendingCount;
+
     // Set the expiration time stamps appropriately.
     void PrepareRecordExpiration(nsHostRecord* rec) const;
 
 public:
     /*
      * Called by the networking dashboard via the DnsService2
      */
     void GetDNSCacheEntries(nsTArray<mozilla::net::DNSCacheEntries> *);