Bug 1320744 - Part 1, make refcounter of HttpChannelChild thread-safe. r=mayhemer draft
authorShih-Chiang Chien <schien@mozilla.com>
Tue, 07 Feb 2017 17:25:45 +0800
changeset 551268 568b0e6178389222197bfbc0093e05fc6cae0794
parent 551223 65b0ac174753b22c01156d72fb42d2abd3176dd1
child 551269 5627acad147bb0881897d93a7eddd2715cf9f3c5
push id51005
push userbmo:schien@mozilla.com
push dateSat, 25 Mar 2017 08:55:30 +0000
reviewersmayhemer
bugs1320744
milestone55.0a1
Bug 1320744 - Part 1, make refcounter of HttpChannelChild thread-safe. r=mayhemer MozReview-Commit-ID: 5Br1WLlpvcR
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.h
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -187,50 +187,64 @@ HttpChannelChild::HttpChannelChild()
   mChannelCreationTimestamp = TimeStamp::Now();
   mAsyncOpenTime = TimeStamp::Now();
   mEventQ = new ChannelEventQueue(static_cast<nsIHttpChannel*>(this));
 }
 
 HttpChannelChild::~HttpChannelChild()
 {
   LOG(("Destroying HttpChannelChild @%p\n", this));
+
+  ReleaseMainThreadOnlyReferences();
 }
 
+void
+HttpChannelChild::ReleaseMainThreadOnlyReferences()
+{
+  if (NS_IsMainThread()) {
+      // Already on main thread, let dtor to
+      // take care of releasing references
+      return;
+  }
+
+  nsTArray<nsCOMPtr<nsISupports>> arrayToRelease;
+  arrayToRelease.AppendElement(mCacheKey.forget());
+
+  NS_DispatchToMainThread(new ProxyReleaseRunnable(Move(arrayToRelease)));
+}
 //-----------------------------------------------------------------------------
 // HttpChannelChild::nsISupports
 //-----------------------------------------------------------------------------
 
-// Override nsHashPropertyBag's AddRef: we don't need thread-safe refcnt
 NS_IMPL_ADDREF(HttpChannelChild)
 
 NS_IMETHODIMP_(MozExternalRefCountType) HttpChannelChild::Release()
 {
-  NS_PRECONDITION(0 != mRefCnt, "dup release");
-  NS_ASSERT_OWNINGTHREAD(HttpChannelChild);
-  --mRefCnt;
-  NS_LOG_RELEASE(this, mRefCnt, "HttpChannelChild");
+  nsrefcnt count = --mRefCnt;
+  MOZ_ASSERT(int32_t(count) >= 0, "dup release");
+  NS_LOG_RELEASE(this, count, "HttpChannelChild");
 
   // Normally we Send_delete in OnStopRequest, but when we need to retain the
   // remote channel for security info IPDL itself holds 1 reference, so we
   // Send_delete when refCnt==1.  But if !mIPCOpen, then there's nobody to send
   // to, so we fall through.
-  if (mKeptAlive && mRefCnt == 1 && mIPCOpen) {
+  if (mKeptAlive && count == 1 && mIPCOpen) {
     mKeptAlive = false;
     // We send a message to the parent, which calls SendDelete, and then the
     // child calling Send__delete__() to finally drop the refcount to 0.
-    SendDeletingChannel();
+    TrySendDeletingChannel();
     return 1;
   }
 
-  if (mRefCnt == 0) {
+  if (count == 0) {
     mRefCnt = 1; /* stabilize */
     delete this;
     return 0;
   }
-  return mRefCnt;
+  return count;
 }
 
 NS_INTERFACE_MAP_BEGIN(HttpChannelChild)
   NS_INTERFACE_MAP_ENTRY(nsIRequest)
   NS_INTERFACE_MAP_ENTRY(nsIChannel)
   NS_INTERFACE_MAP_ENTRY(nsIHttpChannel)
   NS_INTERFACE_MAP_ENTRY(nsIHttpChannelInternal)
   NS_INTERFACE_MAP_ENTRY(nsICacheInfoChannel)
@@ -945,17 +959,17 @@ HttpChannelChild::OnStopRequest(const ns
 
   if (mLoadFlags & LOAD_DOCUMENT_URI) {
     // Keep IPDL channel open, but only for updating security info.
     mKeptAlive = true;
     SendDocumentChannelCleanup();
   } else {
     // The parent process will respond by sending a DeleteSelf message and
     // making sure not to send any more messages after that.
-    SendDeletingChannel();
+    TrySendDeletingChannel();
   }
 }
 
 void
 HttpChannelChild::DoPreOnStopRequest(nsresult aStatus)
 {
   LOG(("HttpChannelChild::DoPreOnStopRequest [this=%p status=%" PRIx32 "]\n",
        this, static_cast<uint32_t>(aStatus)));
@@ -1146,17 +1160,17 @@ HttpChannelChild::FailedAsyncOpen(const 
        this, static_cast<uint32_t>(status)));
 
   mStatus = status;
 
   // We're already being called from IPDL, therefore already "async"
   HandleAsyncAbort();
 
   if (mIPCOpen) {
-    SendDeletingChannel();
+    TrySendDeletingChannel();
   }
 }
 
 void
 HttpChannelChild::DoNotifyListenerCleanup()
 {
   LOG(("HttpChannelChild::DoNotifyListenerCleanup [this=%p]\n", this));
 
@@ -2860,16 +2874,30 @@ NS_IMETHODIMP
 HttpChannelChild::GetResponseSynthesized(bool* aSynthesized)
 {
   NS_ENSURE_ARG_POINTER(aSynthesized);
   *aSynthesized = mSynthesizedResponse;
   return NS_OK;
 }
 
 void
+HttpChannelChild::TrySendDeletingChannel()
+{
+  if (NS_IsMainThread()) {
+    Unused << PHttpChannelChild::SendDeletingChannel();
+    return;
+  }
+
+  DebugOnly<nsresult> rv =
+    NS_DispatchToMainThread(
+      NewNonOwningRunnableMethod(this, &HttpChannelChild::TrySendDeletingChannel));
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
+}
+
+void
 HttpChannelChild::OverrideWithSynthesizedResponse(nsAutoPtr<nsHttpResponseHead>& aResponseHead,
                                                   nsIInputStream* aSynthesizedInput,
                                                   InterceptStreamListener* aStreamListener)
 {
   mInterceptListener = aStreamListener;
 
   // Intercepted responses should already be decoded.  If its a redirect,
   // however, we want to respect the encoding of the final result instead.
--- a/netwerk/protocol/http/HttpChannelChild.h
+++ b/netwerk/protocol/http/HttpChannelChild.h
@@ -159,16 +159,24 @@ protected:
 
   MOZ_MUST_USE bool
   GetAssociatedContentSecurity(nsIAssociatedContentSecurity** res = nullptr);
   virtual void DoNotifyListenerCleanup() override;
 
   NS_IMETHOD GetResponseSynthesized(bool* aSynthesized) override;
 
 private:
+  // this section is for main-thread-only object
+  // all the references need to be proxy released on main thread.
+  nsCOMPtr<nsISupports> mCacheKey;
+
+  // Proxy release all members above on main thread.
+  void ReleaseMainThreadOnlyReferences();
+
+private:
 
   class OverrideRunnable : public Runnable {
   public:
     OverrideRunnable(HttpChannelChild* aChannel,
                      HttpChannelChild* aNewChannel,
                      InterceptStreamListener* aListener,
                      nsIInputStream* aInput,
                      nsAutoPtr<nsHttpResponseHead>& aHead);
@@ -207,28 +215,31 @@ private:
   // Override this channel's pending response with a synthesized one. The content will be
   // asynchronously read from the pump.
   void OverrideWithSynthesizedResponse(nsAutoPtr<nsHttpResponseHead>& aResponseHead,
                                        nsIInputStream* aSynthesizedInput,
                                        InterceptStreamListener* aStreamListener);
 
   void ForceIntercepted(nsIInputStream* aSynthesizedInput);
 
+  // Try send DeletingChannel message to parent side. Dispatch an async task to
+  // main thread if invoking on non-main thread.
+  void TrySendDeletingChannel();
+
   RequestHeaderTuples mClientSetRequestHeaders;
   nsCOMPtr<nsIChildChannel> mRedirectChannelChild;
   RefPtr<InterceptStreamListener> mInterceptListener;
   RefPtr<nsInputStreamPump> mSynthesizedResponsePump;
   nsCOMPtr<nsIInputStream> mSynthesizedInput;
   int64_t mSynthesizedStreamLength;
 
   bool mIsFromCache;
   bool mCacheEntryAvailable;
   uint32_t     mCacheExpirationTime;
   nsCString    mCachedCharset;
-  nsCOMPtr<nsISupports> mCacheKey;
 
   nsCString mProtocolVersion;
 
   // If ResumeAt is called before AsyncOpen, we need to send extra data upstream
   bool mSendResumeAt;
 
   bool mIPCOpen;
   bool mKeptAlive;            // IPC kept open, but only for security info