Bug 1343302 - Explicitly pass the nsHttpChannel to mCacheOpenRunnable instead of using a closure, r=valentin
authorMichael Layzell <michael@thelayzells.com>
Tue, 28 Feb 2017 13:32:35 -0500
changeset 374543 c92dc508b3b50cbf159b6470254d7b4291daf111
parent 374542 ed9b6ec1a0e8789b793bccf1426113e1c78f9ad5
child 374544 f3ee1078d7099276195aed0603e12d26dfae1807
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1343302
milestone54.0a1
Bug 1343302 - Explicitly pass the nsHttpChannel to mCacheOpenRunnable instead of using a closure, r=valentin MozReview-Commit-ID: 2jPiUdI1DWN
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -3703,19 +3703,21 @@ nsHttpChannel::OpenCacheEntry(bool isHtt
         }
 
         mCacheOpenWithPriority = cacheEntryOpenFlags & nsICacheStorage::OPEN_PRIORITY;
         mCacheQueueSizeWhenOpen = CacheStorageService::CacheQueueSize(mCacheOpenWithPriority);
 
         if (!mCacheOpenDelay) {
             rv = cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, this);
         } else {
-            mCacheOpenRunnable = NS_NewRunnableFunction([openURI, extension, cacheEntryOpenFlags, cacheStorage, this] () -> void {
-                cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, this);
-            });
+            // We pass `this` explicitly as a parameter due to the raw pointer
+            // to refcounted object in lambda analysis.
+            mCacheOpenFunc = [openURI, extension, cacheEntryOpenFlags, cacheStorage] (nsHttpChannel* self) -> void {
+                cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, self);
+            };
 
             mCacheOpenTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
             // calls nsHttpChannel::Notify after `mCacheOpenDelay` milliseconds
             mCacheOpenTimer->InitWithCallback(this, mCacheOpenDelay, nsITimer::TYPE_ONE_SHOT);
 
         }
         NS_ENSURE_SUCCESS(rv, rv);
     }
@@ -8664,31 +8666,32 @@ NS_IMETHODIMP
 nsHttpChannel::Test_triggerDelayedOpenCacheEntry()
 {
     MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread");
     nsresult rv;
     if (!mCacheOpenDelay) {
         // No delay was set.
         return NS_ERROR_NOT_AVAILABLE;
     }
-    if (!mCacheOpenRunnable) {
+    if (!mCacheOpenFunc) {
         // There should be a runnable.
         return NS_ERROR_FAILURE;
     }
     if (mCacheOpenTimer) {
         rv = mCacheOpenTimer->Cancel();
         if (NS_FAILED(rv)) {
             return rv;
         }
         mCacheOpenTimer = nullptr;
     }
-    nsCOMPtr<nsIRunnable> runnable;
-    mCacheOpenRunnable.swap(runnable);
     mCacheOpenDelay = 0;
-    runnable->Run();
+    // Avoid re-entrancy issues by nulling our mCacheOpenFunc before calling it.
+    std::function<void(nsHttpChannel*)> cacheOpenFunc = nullptr;
+    std::swap(cacheOpenFunc, mCacheOpenFunc);
+    cacheOpenFunc(this);
 
     return NS_OK;
 }
 
 nsresult
 nsHttpChannel::TriggerNetwork(int32_t aTimeout)
 {
     MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread");
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -616,17 +616,17 @@ private:
     RefPtr<ADivertableParentChannel> mParentChannel;
 
     // True if the channel is reading from cache.
     Atomic<bool> mIsReadingFromCache;
 
     // These next members are only used in unit tests to delay the call to
     // cache->AsyncOpenURI in order to race the cache with the network.
     nsCOMPtr<nsITimer> mCacheOpenTimer;
-    nsCOMPtr<nsIRunnable> mCacheOpenRunnable;
+    std::function<void(nsHttpChannel*)> mCacheOpenFunc;
     uint32_t mCacheOpenDelay = 0;
 
     // We need to remember which is the source of the response we are using.
     enum {
         RESPONSE_PENDING,           // response is pending
         RESPONSE_FROM_CACHE,        // response coming from cache. no network.
         RESPONSE_FROM_NETWORK,      // response coming from the network
     } mFirstResponseSource = RESPONSE_PENDING;