Bug 1176034 - MessagePort should force a close() if the structured clone algorithm fails, r=bent
☠☠ backed out by ac56e43537f9 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 23 Jun 2015 15:50:00 -0700
changeset 250058 4f4ceae7be1a481e0df0f53a7e87c34d33561bf5
parent 250057 b10c59539ec8b9ac1b1c9346f2fdc29330bbfd6e
child 250059 7e9fe30a0b568391a017c934481bfabc9ae97eea
push idunknown
push userunknown
push dateunknown
reviewersbent
bugs1176034
milestone41.0a1
Bug 1176034 - MessagePort should force a close() if the structured clone algorithm fails, r=bent
dom/base/PostMessageEvent.cpp
dom/messagechannel/MessagePort.cpp
dom/messagechannel/MessagePort.h
dom/messagechannel/MessagePortParent.cpp
dom/messagechannel/MessagePortParent.h
dom/messagechannel/MessagePortService.cpp
dom/messagechannel/MessagePortService.h
dom/messagechannel/MessagePortUtils.cpp
dom/messagechannel/tests/mochitest.ini
dom/messagechannel/tests/test_messageChannel_forceClose.html
dom/workers/WorkerPrivate.cpp
ipc/glue/BackgroundParentImpl.cpp
ipc/glue/BackgroundParentImpl.h
ipc/glue/PBackground.ipdl
--- a/dom/base/PostMessageEvent.cpp
+++ b/dom/base/PostMessageEvent.cpp
@@ -233,17 +233,23 @@ PostMessageEvent::TransferStructuredClon
 
 /* static */ void
 PostMessageEvent::FreeTransferStructuredClone(uint32_t aTag,
                                               JS::TransferableOwnership aOwnership,
                                               void *aContent,
                                               uint64_t aExtraData,
                                               void* aClosure)
 {
-  // Nothing to do.
+  if (aTag == SCTAG_DOM_MAP_MESSAGEPORT) {
+    MOZ_ASSERT(aClosure);
+    MOZ_ASSERT(!aContent);
+
+    StructuredCloneInfo* scInfo = static_cast<StructuredCloneInfo*>(aClosure);
+    MessagePort::ForceClose(scInfo->event->GetPortIdentifier(aExtraData));
+  }
 }
 
 PostMessageEvent::PostMessageEvent(nsGlobalWindow* aSource,
                                    const nsAString& aCallerOrigin,
                                    nsGlobalWindow* aTargetWindow,
                                    nsIPrincipal* aProvidedPrincipal,
                                    bool aTrustedCaller)
 : mSource(aSource),
--- a/dom/messagechannel/MessagePort.cpp
+++ b/dom/messagechannel/MessagePort.cpp
@@ -16,16 +16,17 @@
 #include "mozilla/dom/MessagePortChild.h"
 #include "mozilla/dom/MessagePortList.h"
 #include "mozilla/dom/PMessagePort.h"
 #include "mozilla/dom/StructuredCloneTags.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/dom/WorkerScope.h"
 #include "mozilla/ipc/BackgroundChild.h"
 #include "mozilla/ipc/PBackgroundChild.h"
+#include "mozilla/unused.h"
 #include "nsContentUtils.h"
 #include "nsGlobalWindow.h"
 #include "nsPresContext.h"
 #include "ScriptSettings.h"
 #include "SharedMessagePortMessage.h"
 
 #include "nsIBFCacheEntry.h"
 #include "nsIDocument.h"
@@ -244,16 +245,60 @@ public:
 
 private:
   ~MessagePortFeature()
   {
     MOZ_COUNT_DTOR(MessagePortFeature);
   }
 };
 
+class ForceCloseHelper final : public nsIIPCBackgroundChildCreateCallback
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  static void ForceClose(const MessagePortIdentifier& aIdentifier)
+  {
+    PBackgroundChild* actor =
+      mozilla::ipc::BackgroundChild::GetForCurrentThread();
+    if (actor) {
+      unused << actor->SendMessagePortForceClose(aIdentifier.uuid(),
+                                                 aIdentifier.destinationUuid(),
+                                                 aIdentifier.sequenceId());
+      return;
+    }
+
+    nsRefPtr<ForceCloseHelper> helper = new ForceCloseHelper(aIdentifier);
+    if (NS_WARN_IF(!mozilla::ipc::BackgroundChild::GetOrCreateForCurrentThread(helper))) {
+      MOZ_CRASH();
+    }
+  }
+
+private:
+  explicit ForceCloseHelper(const MessagePortIdentifier& aIdentifier)
+    : mIdentifier(aIdentifier)
+  {}
+
+  ~ForceCloseHelper() {}
+
+  void ActorFailed() override
+  {
+    MOZ_CRASH("Failed to create a PBackgroundChild actor!");
+  }
+
+  void ActorCreated(mozilla::ipc::PBackgroundChild* aActor)
+  {
+    ForceClose(mIdentifier);
+  }
+
+  const MessagePortIdentifier mIdentifier;
+};
+
+NS_IMPL_ISUPPORTS(ForceCloseHelper, nsIIPCBackgroundChildCreateCallback)
+
 } // anonymous namespace
 
 MessagePort::MessagePort(nsPIDOMWindow* aWindow)
   : MessagePortBase(aWindow)
   , mInnerID(0)
   , mMessageQueueEnabled(false)
   , mIsKeptAlive(false)
 {
@@ -858,10 +903,16 @@ MessagePort::RemoveDocFromBFCache()
   nsCOMPtr<nsIBFCacheEntry> bfCacheEntry = doc->GetBFCacheEntry();
   if (!bfCacheEntry) {
     return;
   }
 
   bfCacheEntry->RemoveFromBFCacheSync();
 }
 
+/* static */ void
+MessagePort::ForceClose(const MessagePortIdentifier& aIdentifier)
+{
+  ForceCloseHelper::ForceClose(aIdentifier);
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/messagechannel/MessagePort.h
+++ b/dom/messagechannel/MessagePort.h
@@ -81,16 +81,19 @@ public:
   static already_AddRefed<MessagePort>
   Create(nsPIDOMWindow* aWindow, const nsID& aUUID,
          const nsID& aDestinationUUID, ErrorResult& aRv);
 
   static already_AddRefed<MessagePort>
   Create(nsPIDOMWindow* aWindow, const MessagePortIdentifier& aIdentifier,
          ErrorResult& aRv);
 
+  static void
+  ForceClose(const MessagePortIdentifier& aIdentifier);
+
   virtual JSObject*
   WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   virtual void
   PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
               const Optional<Sequence<JS::Value>>& aTransferable,
               ErrorResult& aRv) override;
 
--- a/dom/messagechannel/MessagePortParent.cpp
+++ b/dom/messagechannel/MessagePortParent.cpp
@@ -154,10 +154,24 @@ MessagePortParent::CloseAndDelete()
 
 void
 MessagePortParent::Close()
 {
   mService = nullptr;
   mEntangled = false;
 }
 
+/* static */ bool
+MessagePortParent::ForceClose(const nsID& aUUID,
+                              const nsID& aDestinationUUID,
+                              const uint32_t& aSequenceID)
+{
+  MessagePortService* service =  MessagePortService::Get();
+  if (!service) {
+    NS_WARNING("The service must exist if we want to close an existing MessagePort.");
+    return false;
+  }
+
+  return service->ForceClose(aUUID, aDestinationUUID, aSequenceID);
+}
+
 } // dom namespace
 } // mozilla namespace
--- a/dom/messagechannel/MessagePortParent.h
+++ b/dom/messagechannel/MessagePortParent.h
@@ -31,16 +31,20 @@ public:
     return mCanSendData;
   }
 
   const nsID& ID() const
   {
     return mUUID;
   }
 
+  static bool ForceClose(const nsID& aUUID,
+                         const nsID& aDestinationUUID,
+                         const uint32_t& aSequenceID);
+
 private:
   virtual bool RecvPostMessages(nsTArray<MessagePortMessage>&& aMessages)
                                                                        override;
 
   virtual bool RecvDisentangle(nsTArray<MessagePortMessage>&& aMessages)
                                                                        override;
 
   virtual bool RecvStopSendingData() override;
--- a/dom/messagechannel/MessagePortService.cpp
+++ b/dom/messagechannel/MessagePortService.cpp
@@ -1,26 +1,37 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "MessagePortService.h"
 #include "MessagePortParent.h"
 #include "SharedMessagePortMessage.h"
+#include "mozilla/ipc/BackgroundParent.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/unused.h"
 #include "nsDataHashtable.h"
 #include "nsTArray.h"
 
+using mozilla::ipc::AssertIsOnBackgroundThread;
+
 namespace mozilla {
 namespace dom {
 
 namespace {
+
 StaticRefPtr<MessagePortService> gInstance;
+
+void
+AssertIsInMainProcess()
+{
+  MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
+}
+
 } // anonymous namespace
 
 class MessagePortService::MessagePortServiceData final
 {
 public:
   explicit MessagePortServiceData(const nsID& aDestinationUUID)
     : mDestinationUUID(aDestinationUUID)
     , mSequenceID(1)
@@ -49,18 +60,30 @@ public:
     MessagePortParent* mParent;
   };
 
   FallibleTArray<NextParent> mNextParents;
   FallibleTArray<nsRefPtr<SharedMessagePortMessage>> mMessages;
 };
 
 /* static */ MessagePortService*
+MessagePortService::Get()
+{
+  AssertIsInMainProcess();
+  AssertIsOnBackgroundThread();
+
+  return gInstance;
+}
+
+/* static */ MessagePortService*
 MessagePortService::GetOrCreate()
 {
+  AssertIsInMainProcess();
+  AssertIsOnBackgroundThread();
+
   if (!gInstance) {
     gInstance = new MessagePortService();
   }
 
   return gInstance;
 }
 
 bool
@@ -243,16 +266,22 @@ MessagePortService::CloseAll(const nsID&
     parent.mParent->CloseAndDelete();
   }
 
   nsID destinationUUID = data->mDestinationUUID;
   mPorts.Remove(aUUID);
 
   CloseAll(destinationUUID);
 
+  // CloseAll calls itself recursively and it can happen that it deletes
+  // itself. Before continuing we must check if we are still alive.
+  if (!gInstance) {
+    return;
+  }
+
 #ifdef DEBUG
   mPorts.EnumerateRead(CloseAllDebugCheck, const_cast<nsID*>(&aUUID));
 #endif
 
   MaybeShutdown();
 }
 
 // This service can be dismissed when there are not active ports.
@@ -319,10 +348,31 @@ MessagePortService::ParentDestroy(Messag
        break;
       }
     }
   }
 
   CloseAll(aParent->ID());
 }
 
+bool
+MessagePortService::ForceClose(const nsID& aUUID,
+                               const nsID& aDestinationUUID,
+                               const uint32_t& aSequenceID)
+{
+  MessagePortServiceData* data;
+  if (!mPorts.Get(aUUID, &data)) {
+    NS_WARNING("Unknown MessagePort in ForceClose()");
+    return true;
+  }
+
+  if (!data->mDestinationUUID.Equals(aDestinationUUID) ||
+      data->mSequenceID != aSequenceID) {
+    NS_WARNING("DestinationUUID and/or sequenceID do not match.");
+    return false;
+  }
+
+  CloseAll(aUUID);
+  return true;
+}
+
 } // dom namespace
 } // mozilla namespace
--- a/dom/messagechannel/MessagePortService.h
+++ b/dom/messagechannel/MessagePortService.h
@@ -15,16 +15,17 @@ namespace dom {
 class MessagePortParent;
 class SharedMessagePortMessage;
 
 class MessagePortService final
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(MessagePortService)
 
+  static MessagePortService* Get();
   static MessagePortService* GetOrCreate();
 
   bool RequestEntangling(MessagePortParent* aParent,
                          const nsID& aDestinationUUID,
                          const uint32_t& aSequenceID);
 
   bool DisentanglePort(
                  MessagePortParent* aParent,
@@ -33,16 +34,20 @@ public:
   bool ClosePort(MessagePortParent* aParent);
 
   bool PostMessages(
                  MessagePortParent* aParent,
                  FallibleTArray<nsRefPtr<SharedMessagePortMessage>>& aMessages);
 
   void ParentDestroy(MessagePortParent* aParent);
 
+  bool ForceClose(const nsID& aUUID,
+                  const nsID& aDestinationUUID,
+                  const uint32_t& aSequenceID);
+
 private:
   ~MessagePortService() {}
 
   void CloseAll(const nsID& aUUID);
   void MaybeShutdown();
 
   class MessagePortServiceData;
 
--- a/dom/messagechannel/MessagePortUtils.cpp
+++ b/dom/messagechannel/MessagePortUtils.cpp
@@ -136,17 +136,17 @@ ReadTransfer(JSContext* aCx, JSStructure
   }
 
   MOZ_ASSERT(aContent == 0);
   MOZ_ASSERT(aExtraData < closure->mClosure.mMessagePortIdentifiers.Length());
 
   ErrorResult rv;
   nsRefPtr<MessagePort> port =
     MessagePort::Create(closure->mWindow,
-                        closure->mClosure.mMessagePortIdentifiers[(uint32_t)aExtraData],
+                        closure->mClosure.mMessagePortIdentifiers[aExtraData],
                         rv);
   if (NS_WARN_IF(rv.Failed())) {
     return false;
   }
 
   closure->mMessagePorts.AppendElement(port);
 
   JS::Rooted<JS::Value> value(aCx);
@@ -190,23 +190,37 @@ WriteTransfer(JSContext* aCx, JS::Handle
   *aTag = SCTAG_DOM_MAP_MESSAGEPORT;
   *aOwnership = JS::SCTAG_TMO_CUSTOM;
   *aContent = nullptr;
   *aExtraData = closure->mClosure.mMessagePortIdentifiers.Length() - 1;
 
   return true;
 }
 
+void
+FreeTransfer(uint32_t aTag, JS::TransferableOwnership aOwnership,
+             void* aContent, uint64_t aExtraData, void* aClosure)
+{
+  MOZ_ASSERT(aClosure);
+  auto* closure = static_cast<StructuredCloneClosureInternal*>(aClosure);
+
+  if (aTag == SCTAG_DOM_MAP_MESSAGEPORT) {
+    MOZ_ASSERT(!aContent);
+    MOZ_ASSERT(aExtraData < closure->mClosure.mMessagePortIdentifiers.Length());
+    MessagePort::ForceClose(closure->mClosure.mMessagePortIdentifiers[(uint32_t)aExtraData]);
+  }
+}
+
 const JSStructuredCloneCallbacks gCallbacks = {
   Read,
   Write,
   Error,
   ReadTransfer,
   WriteTransfer,
-  nullptr
+  FreeTransfer,
 };
 
 } // anonymous namespace
 
 bool
 ReadStructuredCloneWithTransfer(JSContext* aCx, nsTArray<uint8_t>& aData,
                                 const StructuredCloneClosure& aClosure,
                                 JS::MutableHandle<JS::Value> aClone,
--- a/dom/messagechannel/tests/mochitest.ini
+++ b/dom/messagechannel/tests/mochitest.ini
@@ -18,8 +18,9 @@ support-files =
 [test_messageChannel_start.html]
 [test_messageChannel_transferable.html]
 [test_messageChannel_unshipped.html]
 [test_messageChannel_worker.html]
 [test_messageChannel_selfTransferring.html]
 [test_messageChannel_sharedWorker.html]
 [test_messageChannel_sharedWorker2.html]
 [test_messageChannel_any.html]
+[test_messageChannel_forceClose.html]
new file mode 100644
--- /dev/null
+++ b/dom/messagechannel/tests/test_messageChannel_forceClose.html
@@ -0,0 +1,36 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1176034
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1176034 - start/close</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1176034">Mozilla Bug 1176034</a>
+<div id="content"></div>
+<pre id="test">
+</pre>
+  <script type="application/javascript">
+
+  function runTests() {
+    var mc = new MessageChannel();
+
+    try {
+      postMessage(42, "*", [ mc.port1, window ]);
+      ok(false, "Something went wrong.");
+    } catch(e) {
+      ok(true, "PostMessage should fail and we should not leak.");
+    }
+
+    SimpleTest.finish();
+  }
+
+  SimpleTest.waitForExplicitFinish();
+  SpecialPowers.pushPrefEnv({"set": [["dom.messageChannel.enabled", true]]}, runTests);
+  </script>
+</body>
+</html>
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -758,17 +758,24 @@ struct WorkerStructuredCloneCallbacks
 
     return false;
   }
 
   static void
   FreeTransfer(uint32_t aTag, JS::TransferableOwnership aOwnership,
                void *aContent, uint64_t aExtraData, void* aClosure)
   {
-    // Nothing to do.
+    if (aTag == SCTAG_DOM_MAP_MESSAGEPORT) {
+      MOZ_ASSERT(aClosure);
+      MOZ_ASSERT(!aContent);
+      auto* closure = static_cast<WorkerStructuredCloneClosure*>(aClosure);
+
+      MOZ_ASSERT(aExtraData < closure->mMessagePortIdentifiers.Length());
+      dom::MessagePort::ForceClose(closure->mMessagePortIdentifiers[aExtraData]);
+    }
   }
 };
 
 const JSStructuredCloneCallbacks gWorkerStructuredCloneCallbacks = {
   WorkerStructuredCloneCallbacks::Read,
   WorkerStructuredCloneCallbacks::Write,
   WorkerStructuredCloneCallbacks::Error,
   WorkerStructuredCloneCallbacks::ReadTransfer,
--- a/ipc/glue/BackgroundParentImpl.cpp
+++ b/ipc/glue/BackgroundParentImpl.cpp
@@ -583,16 +583,27 @@ BackgroundParentImpl::DeallocPMessagePor
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aActor);
 
   delete static_cast<MessagePortParent*>(aActor);
   return true;
 }
 
+bool
+BackgroundParentImpl::RecvMessagePortForceClose(const nsID& aUUID,
+                                                const nsID& aDestinationUUID,
+                                                const uint32_t& aSequenceID)
+{
+  AssertIsInMainProcess();
+  AssertIsOnBackgroundThread();
+
+  return MessagePortParent::ForceClose(aUUID, aDestinationUUID, aSequenceID);
+}
+
 } // namespace ipc
 } // namespace mozilla
 
 void
 TestParent::ActorDestroy(ActorDestroyReason aWhy)
 {
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
--- a/ipc/glue/BackgroundParentImpl.h
+++ b/ipc/glue/BackgroundParentImpl.h
@@ -132,14 +132,19 @@ protected:
   virtual bool
   RecvPMessagePortConstructor(PMessagePortParent* aActor,
                               const nsID& aUUID,
                               const nsID& aDestinationUUID,
                               const uint32_t& aSequenceID) override;
 
   virtual bool
   DeallocPMessagePortParent(PMessagePortParent* aActor) override;
+
+  virtual bool
+  RecvMessagePortForceClose(const nsID& aUUID,
+                            const nsID& aDestinationUUID,
+                            const uint32_t& aSequenceID) override;
 };
 
 } // namespace ipc
 } // namespace mozilla
 
 #endif // mozilla_ipc_backgroundparentimpl_h__
--- a/ipc/glue/PBackground.ipdl
+++ b/ipc/glue/PBackground.ipdl
@@ -55,16 +55,18 @@ parent:
   PServiceWorkerManager();
 
   ShutdownServiceWorkerRegistrar();
 
   PCacheStorage(Namespace aNamespace, PrincipalInfo aPrincipalInfo);
 
   PMessagePort(nsID uuid, nsID destinationUuid, uint32_t sequenceId);
 
+  MessagePortForceClose(nsID uuid, nsID destinationUuid, uint32_t sequenceId);
+
 child:
   PCache();
   PCacheStreamControl();
 
 both:
   PBlob(BlobConstructorParams params);
 
   PFileDescriptorSet(FileDescriptor fd);