Bug 1369571 - break reference cycle between HttpChannelChild and HttpBackgroundChannelChild while PHttpChannel is destroyed. r?mayhemer draft
authorShih-Chiang Chien <schien@mozilla.com>
Tue, 18 Jul 2017 19:03:14 +0800
changeset 612127 b4ca5e3b477185fb0ac785410895ad952b730e33
parent 610973 1b065ffd8a535a0ad4c39a912af18e948e6a42c1
child 638316 fb6176dbbbe95547ab0b5717bf3930c03de096d6
push id69390
push userschien@mozilla.com
push dateThu, 20 Jul 2017 10:16:12 +0000
reviewersmayhemer
bugs1369571
milestone56.0a1
Bug 1369571 - break reference cycle between HttpChannelChild and HttpBackgroundChannelChild while PHttpChannel is destroyed. r?mayhemer The reference cycle is supposed to be removed when HttpBackgroundChannelChild::ActorDestroyed, however this operation is pending due until HttpChannelChild receives OnStartRequest IPC message. Unfortunately the OnStartRequest IPC message is dropped because of PContent shutdown procedure. So, the reference removal is never executed, which leaks memory until content process is terminated. MozReview-Commit-ID: 7R6nt7W690o
netwerk/protocol/http/HttpBackgroundChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.h
--- a/netwerk/protocol/http/HttpBackgroundChannelChild.cpp
+++ b/netwerk/protocol/http/HttpBackgroundChannelChild.cpp
@@ -114,16 +114,19 @@ HttpBackgroundChannelChild::Init(HttpCha
 void
 HttpBackgroundChannelChild::OnChannelClosed()
 {
   LOG(("HttpBackgroundChannelChild::OnChannelClosed [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
 
   // HttpChannelChild is not going to handle any incoming message.
   mChannelChild = nullptr;
+
+  // Remove pending IPC messages as well.
+  mQueuedRunnables.Clear();
 }
 
 void
 HttpBackgroundChannelChild::OnStartRequestReceived()
 {
   LOG(("HttpBackgroundChannelChild::OnStartRequestReceived [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
   MOZ_ASSERT(mChannelChild);
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -3604,10 +3604,23 @@ HttpChannelChild::ShouldInterceptURI(nsI
 
 mozilla::ipc::IPCResult
 HttpChannelChild::RecvSetPriority(const int16_t& aPriority)
 {
   mPriority = aPriority;
   return IPC_OK();
 }
 
+void
+HttpChannelChild::ActorDestroy(ActorDestroyReason aWhy)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // OnStartRequest might be dropped if IPDL is destroyed abnormally
+  // and BackgroundChild might have pending IPC messages.
+  // Clean up BackgroundChild at this time to prevent memleak.
+  if (aWhy != Deletion) {
+    CleanupBackgroundChannel();
+  }
+}
+
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/http/HttpChannelChild.h
+++ b/netwerk/protocol/http/HttpChannelChild.h
@@ -160,16 +160,18 @@ protected:
   mozilla::ipc::IPCResult RecvReportSecurityMessage(const nsString& messageTag,
                                                     const nsString& messageCategory) override;
 
   mozilla::ipc::IPCResult RecvIssueDeprecationWarning(const uint32_t& warning,
                                                       const bool& asError) override;
 
   mozilla::ipc::IPCResult RecvSetPriority(const int16_t& aPriority) override;
 
+  virtual void ActorDestroy(ActorDestroyReason aWhy) override;
+
   MOZ_MUST_USE bool
   GetAssociatedContentSecurity(nsIAssociatedContentSecurity** res = nullptr);
   virtual void DoNotifyListenerCleanup() override;
 
   NS_IMETHOD GetResponseSynthesized(bool* aSynthesized) override;
 
   nsresult
   AsyncCall(void (HttpChannelChild::*funcPtr)(),