Bug 1123021 - Better life-time management for WebSocketChannelChild. r=smaug, a=sledru
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 19 Jan 2015 17:21:20 +0000
changeset 249374 0fe217418754b6aeef7d4d3fd733ee4088ae60ea
parent 249373 c0f368aff73869659019b19a48cdf2fdd28b92d8
child 249375 9aed7f7c9de8a189998e554b5febf2015f43b298
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, sledru
bugs1123021
milestone37.0a2
Bug 1123021 - Better life-time management for WebSocketChannelChild. r=smaug, a=sledru
netwerk/protocol/websocket/WebSocketChannelChild.cpp
netwerk/protocol/websocket/WebSocketChannelChild.h
--- a/netwerk/protocol/websocket/WebSocketChannelChild.cpp
+++ b/netwerk/protocol/websocket/WebSocketChannelChild.cpp
@@ -24,18 +24,18 @@ namespace net {
 NS_IMPL_ADDREF(WebSocketChannelChild)
 
 NS_IMETHODIMP_(MozExternalRefCountType) WebSocketChannelChild::Release()
 {
   NS_PRECONDITION(0 != mRefCnt, "dup release");
   --mRefCnt;
   NS_LOG_RELEASE(this, mRefCnt, "WebSocketChannelChild");
 
-  if (mRefCnt == 1 && mIPCOpen) {
-    SendDeleteSelf();
+  if (mRefCnt == 1) {
+    MaybeReleaseIPCObject();
     return mRefCnt;
   }
 
   if (mRefCnt == 0) {
     mRefCnt = 1; /* stabilize */
     delete this;
     return 0;
   }
@@ -45,47 +45,82 @@ NS_IMETHODIMP_(MozExternalRefCountType) 
 NS_INTERFACE_MAP_BEGIN(WebSocketChannelChild)
   NS_INTERFACE_MAP_ENTRY(nsIWebSocketChannel)
   NS_INTERFACE_MAP_ENTRY(nsIProtocolHandler)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIWebSocketChannel)
   NS_INTERFACE_MAP_ENTRY(nsIThreadRetargetableRequest)
 NS_INTERFACE_MAP_END
 
 WebSocketChannelChild::WebSocketChannelChild(bool aEncrypted)
- : mIPCOpen(false)
+ : mIPCState(Closed)
+ , mMutex("WebSocketChannelChild::mMutex")
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread");
 
   LOG(("WebSocketChannelChild::WebSocketChannelChild() %p\n", this));
   mEncrypted = aEncrypted;
   mEventQ = new ChannelEventQueue(static_cast<nsIWebSocketChannel*>(this));
 }
 
 WebSocketChannelChild::~WebSocketChannelChild()
 {
   LOG(("WebSocketChannelChild::~WebSocketChannelChild() %p\n", this));
 }
 
 void
 WebSocketChannelChild::AddIPDLReference()
 {
-  NS_ABORT_IF_FALSE(!mIPCOpen, "Attempt to retain more than one IPDL reference");
-  mIPCOpen = true;
+  MOZ_ASSERT(NS_IsMainThread());
+
+  {
+    MutexAutoLock lock(mMutex);
+    NS_ABORT_IF_FALSE(mIPCState == Closed, "Attempt to retain more than one IPDL reference");
+    mIPCState = Opened;
+  }
+
   AddRef();
 }
 
 void
 WebSocketChannelChild::ReleaseIPDLReference()
 {
-  NS_ABORT_IF_FALSE(mIPCOpen, "Attempt to release nonexistent IPDL reference");
-  mIPCOpen = false;
+  MOZ_ASSERT(NS_IsMainThread());
+
+  {
+    MutexAutoLock lock(mMutex);
+    NS_ABORT_IF_FALSE(mIPCState != Closed, "Attempt to release nonexistent IPDL reference");
+    mIPCState = Closed;
+  }
+
   Release();
 }
 
 void
+WebSocketChannelChild::MaybeReleaseIPCObject()
+{
+  {
+    MutexAutoLock lock(mMutex);
+    if (mIPCState != Opened) {
+      return;
+    }
+
+    mIPCState = Closing;
+  }
+
+  if (!NS_IsMainThread()) {
+    nsCOMPtr<nsIRunnable> runnable =
+      NS_NewRunnableMethod(this, &WebSocketChannelChild::MaybeReleaseIPCObject);
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(runnable)));
+    return;
+  }
+
+  SendDeleteSelf();
+}
+
+void
 WebSocketChannelChild::GetEffectiveURL(nsAString& aEffectiveURL) const
 {
   aEffectiveURL = mEffectiveURL;
 }
 
 bool
 WebSocketChannelChild::IsEncrypted() const
 {
@@ -160,17 +195,17 @@ class StartEvent : public ChannelEvent
   , mEncrypted(aEncrypted)
   {}
 
   void Run()
   {
     mChild->OnStart(mProtocol, mExtensions, mEffectiveURL, mEncrypted);
   }
  private:
-  WebSocketChannelChild* mChild;
+  nsRefPtr<WebSocketChannelChild> mChild;
   nsCString mProtocol;
   nsCString mExtensions;
   nsString mEffectiveURL;
   bool mEncrypted;
 };
 
 bool
 WebSocketChannelChild::RecvOnStart(const nsCString& aProtocol,
@@ -219,17 +254,17 @@ class StopEvent : public ChannelEvent
   , mStatusCode(aStatusCode)
   {}
 
   void Run()
   {
     mChild->OnStop(mStatusCode);
   }
  private:
-  WebSocketChannelChild* mChild;
+  nsRefPtr<WebSocketChannelChild> mChild;
   nsresult mStatusCode;
 };
 
 bool
 WebSocketChannelChild::RecvOnStop(const nsresult& aStatusCode)
 {
   if (mEventQ->ShouldEnqueue()) {
     mEventQ->Enqueue(new EventTargetDispatcher(
@@ -267,17 +302,17 @@ class MessageEvent : public ChannelEvent
   {
     if (!mBinary) {
       mChild->OnMessageAvailable(mMessage);
     } else {
       mChild->OnBinaryMessageAvailable(mMessage);
     }
   }
  private:
-  WebSocketChannelChild* mChild;
+  nsRefPtr<WebSocketChannelChild> mChild;
   nsCString mMessage;
   bool mBinary;
 };
 
 bool
 WebSocketChannelChild::RecvOnMessageAvailable(const nsCString& aMsg)
 {
   if (mEventQ->ShouldEnqueue()) {
@@ -335,17 +370,17 @@ class AcknowledgeEvent : public ChannelE
   , mSize(aSize)
   {}
 
   void Run()
   {
     mChild->OnAcknowledge(mSize);
   }
  private:
-  WebSocketChannelChild* mChild;
+  nsRefPtr<WebSocketChannelChild> mChild;
   uint32_t mSize;
 };
 
 bool
 WebSocketChannelChild::RecvOnAcknowledge(const uint32_t& aSize)
 {
   if (mEventQ->ShouldEnqueue()) {
     mEventQ->Enqueue(new EventTargetDispatcher(
@@ -379,19 +414,19 @@ class ServerCloseEvent : public ChannelE
   , mReason(aReason)
   {}
 
   void Run()
   {
     mChild->OnServerClose(mCode, mReason);
   }
  private:
-  WebSocketChannelChild* mChild;
-  uint16_t               mCode;
-  nsCString              mReason;
+  nsRefPtr<WebSocketChannelChild> mChild;
+  uint16_t mCode;
+  nsCString mReason;
 };
 
 bool
 WebSocketChannelChild::RecvOnServerClose(const uint16_t& aCode,
                                          const nsCString& aReason)
 {
   if (mEventQ->ShouldEnqueue()) {
     mEventQ->Enqueue(new EventTargetDispatcher(
@@ -492,18 +527,27 @@ NS_IMETHODIMP
 WebSocketChannelChild::Close(uint16_t code, const nsACString & reason)
 {
   if (!NS_IsMainThread()) {
     MOZ_RELEASE_ASSERT(NS_GetCurrentThread() == mTargetThread);
     return NS_DispatchToMainThread(new CloseEvent(this, code, reason));
   }
   LOG(("WebSocketChannelChild::Close() %p\n", this));
 
-  if (!mIPCOpen || !SendClose(code, nsCString(reason)))
+  {
+    MutexAutoLock lock(mMutex);
+    if (mIPCState != Opened) {
+      return NS_ERROR_UNEXPECTED;
+    }
+  }
+
+  if (!SendClose(code, nsCString(reason))) {
     return NS_ERROR_UNEXPECTED;
+  }
+
   return NS_OK;
 }
 
 class MsgEvent : public nsRunnable
 {
 public:
   MsgEvent(WebSocketChannelChild *aChild,
            const nsACString &aMsg,
@@ -535,32 +579,50 @@ NS_IMETHODIMP
 WebSocketChannelChild::SendMsg(const nsACString &aMsg)
 {
   if (!NS_IsMainThread()) {
     MOZ_RELEASE_ASSERT(IsOnTargetThread());
     return NS_DispatchToMainThread(new MsgEvent(this, aMsg, false));
   }
   LOG(("WebSocketChannelChild::SendMsg() %p\n", this));
 
-  if (!mIPCOpen || !SendSendMsg(nsCString(aMsg)))
+  {
+    MutexAutoLock lock(mMutex);
+    if (mIPCState != Opened) {
+      return NS_ERROR_UNEXPECTED;
+    }
+  }
+
+  if (!SendSendMsg(nsCString(aMsg))) {
     return NS_ERROR_UNEXPECTED;
+  }
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 WebSocketChannelChild::SendBinaryMsg(const nsACString &aMsg)
 {
   if (!NS_IsMainThread()) {
     MOZ_RELEASE_ASSERT(IsOnTargetThread());
     return NS_DispatchToMainThread(new MsgEvent(this, aMsg, true));
   }
   LOG(("WebSocketChannelChild::SendBinaryMsg() %p\n", this));
 
-  if (!mIPCOpen || !SendSendBinaryMsg(nsCString(aMsg)))
+  {
+    MutexAutoLock lock(mMutex);
+    if (mIPCState != Opened) {
+      return NS_ERROR_UNEXPECTED;
+    }
+  }
+
+  if (!SendSendBinaryMsg(nsCString(aMsg))) {
     return NS_ERROR_UNEXPECTED;
+  }
+
   return NS_OK;
 }
 
 class BinaryStreamEvent : public nsRunnable
 {
 public:
   BinaryStreamEvent(WebSocketChannelChild *aChild,
                     OptionalInputStreamParams *aStream,
@@ -604,18 +666,27 @@ WebSocketChannelChild::SendBinaryStream(
 nsresult
 WebSocketChannelChild::SendBinaryStream(OptionalInputStreamParams *aStream,
                                         uint32_t aLength)
 {
   LOG(("WebSocketChannelChild::SendBinaryStream() %p\n", this));
 
   nsAutoPtr<OptionalInputStreamParams> stream(aStream);
 
-  if (!mIPCOpen || !SendSendBinaryStream(*stream, aLength))
+  {
+    MutexAutoLock lock(mMutex);
+    if (mIPCState != Opened) {
+      return NS_ERROR_UNEXPECTED;
+    }
+  }
+
+  if (!SendSendBinaryStream(*stream, aLength)) {
     return NS_ERROR_UNEXPECTED;
+  }
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 WebSocketChannelChild::GetSecurityInfo(nsISupports **aSecurityInfo)
 {
   LOG(("WebSocketChannelChild::GetSecurityInfo() %p\n", this));
   return NS_ERROR_NOT_AVAILABLE;
--- a/netwerk/protocol/websocket/WebSocketChannelChild.h
+++ b/netwerk/protocol/websocket/WebSocketChannelChild.h
@@ -62,19 +62,29 @@ class WebSocketChannelChild : public Bas
   void OnBinaryMessageAvailable(const nsCString& aMsg);
   void OnAcknowledge(const uint32_t& aSize);
   void OnServerClose(const uint16_t& aCode, const nsCString& aReason);
   void AsyncOpenFailed();  
 
   void DispatchToTargetThread(ChannelEvent *aChannelEvent);
   bool IsOnTargetThread();
 
+  void MaybeReleaseIPCObject();
+
   nsRefPtr<ChannelEventQueue> mEventQ;
   nsString mEffectiveURL;
-  bool mIPCOpen;
+
+  // This variable is protected by mutex.
+  enum {
+    Opened,
+    Closing,
+    Closed
+  } mIPCState;
+
+  mozilla::Mutex mMutex;
 
   friend class StartEvent;
   friend class StopEvent;
   friend class MessageEvent;
   friend class AcknowledgeEvent;
   friend class ServerCloseEvent;
   friend class AsyncOpenFailedEvent;
 };