Bug 1224825 - Race condition in MessagePort::close - patch 1, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 17 Nov 2015 23:38:01 +0000
changeset 273055 f04ec936e2260c65189300b934f0fdb7ca7f18a2
parent 273054 50c1b2cb6ad1141a2bd53ed8b8c1738b42cf3a45
child 273056 67a3836250dee8e28433881e9866a6328dc0de44
push id29693
push usercbook@mozilla.com
push dateWed, 18 Nov 2015 13:50:33 +0000
treeherdermozilla-central@1d6155d7e6c9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1224825
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1224825 - Race condition in MessagePort::close - patch 1, r=smaug
dom/messagechannel/MessagePort.cpp
dom/messagechannel/MessagePort.h
dom/messagechannel/MessagePortService.cpp
dom/messagechannel/tests/mochitest.ini
dom/messagechannel/tests/test_messageChannel_bug1224825.html
dom/messagechannel/tests/test_messageChannel_worker_forceClose.html
--- a/dom/messagechannel/MessagePort.cpp
+++ b/dom/messagechannel/MessagePort.cpp
@@ -144,16 +144,20 @@ public:
 
     RefPtr<MessagePortList> portList =
       new MessagePortList(static_cast<dom::Event*>(event.get()),
                           ports);
     event->SetPorts(portList);
 
     bool dummy;
     mPort->DispatchEvent(static_cast<dom::Event*>(event.get()), &dummy);
+
+    // We must check if we were waiting for this message in order to shutdown
+    // the port.
+    mPort->UpdateMustKeepAlive();
     return NS_OK;
   }
 
   NS_IMETHOD
   Cancel() override
   {
     mPort = nullptr;
     mData = nullptr;
@@ -211,18 +215,20 @@ public:
     : mPort(aPort)
   {
     MOZ_ASSERT(aPort);
     MOZ_COUNT_CTOR(MessagePortFeature);
   }
 
   virtual bool Notify(JSContext* aCx, workers::Status aStatus) override
   {
-    if (mPort && aStatus > Running) {
-      mPort->Close();
+    if (aStatus > Running) {
+      // We cannot process messages anymore because we cannot dispatch new
+      // runnables. Let's force a Close().
+      mPort->CloseForced();
     }
 
     return true;
   }
 
 private:
   ~MessagePortFeature()
   {
@@ -331,17 +337,16 @@ MessagePort::Initialize(const nsID& aUUI
                         State aState, ErrorResult& aRv)
 {
   MOZ_ASSERT(mIdentifier);
   mIdentifier->uuid() = aUUID;
   mIdentifier->destinationUuid() = aDestinationUUID;
   mIdentifier->sequenceId() = aSequenceID;
 
   mState = aState;
-  mNextStep = eNextStepNone;
 
   if (mNeutered) {
     // If this port is neutered we don't want to keep it alive artificially nor
     // we want to add listeners or workerFeatures.
     mState = eStateDisentangled;
     return;
   }
 
@@ -447,18 +452,19 @@ MessagePort::PostMessage(JSContext* aCx,
   // If we are unshipped we are connected to the other port on the same thread.
   if (mState == eStateUnshippedEntangled) {
     MOZ_ASSERT(mUnshippedEntangledPort);
     mUnshippedEntangledPort->mMessages.AppendElement(data);
     mUnshippedEntangledPort->Dispatch();
     return;
   }
 
-  // Not entangled yet, but already closed.
-  if (mNextStep != eNextStepNone) {
+  // Not entangled yet, but already closed/disentangled.
+  if (mState == eStateEntanglingForDisentangle ||
+      mState == eStateEntanglingForClose) {
     return;
   }
 
   RemoveDocFromBFCache();
 
   // Not entangled yet.
   if (mState == eStateEntangling) {
     mMessagesForTheOtherPort.AppendElement(data);
@@ -485,63 +491,131 @@ MessagePort::Start()
 
   mMessageQueueEnabled = true;
   Dispatch();
 }
 
 void
 MessagePort::Dispatch()
 {
-  if (!mMessageQueueEnabled || mMessages.IsEmpty() || mDispatchRunnable ||
-      mState > eStateEntangled || mNextStep != eNextStepNone) {
+  if (!mMessageQueueEnabled || mMessages.IsEmpty() || mDispatchRunnable) {
     return;
   }
 
+  switch (mState) {
+    case eStateUnshippedEntangled:
+      // Everything is fine here. We have messages because the other
+      // port populates our queue directly.
+      break;
+
+    case eStateEntangling:
+      // Everything is fine here as well. We have messages because the other
+      // port populated our queue directly when we were in the
+      // eStateUnshippedEntangled state.
+      break;
+
+    case eStateEntanglingForDisentangle:
+      // Here we don't want to ship messages because these messages must be
+      // delivered by the cloned version of this one. They will be sent in the
+      // SendDisentangle().
+      return;
+
+    case eStateEntanglingForClose:
+      // We still want to deliver messages if we are closing. These messages
+      // are here from the previous eStateUnshippedEntangled state.
+      break;
+
+    case eStateEntangled:
+      // This port is up and running.
+      break;
+
+    case eStateDisentangling:
+      // If we are in the process to disentangle the port, we cannot dispatch
+      // messages. They will be sent to the cloned version of this port via
+      // SendDisentangle();
+      return;
+
+    case eStateDisentangled:
+      MOZ_CRASH("This cannot happen.");
+      // It cannot happen because Disentangle should take off all the pending
+      // messages.
+      break;
+
+    case eStateDisentangledForClose:
+      // If we are here is because the port has been closed. We can still
+      // process the pending messages.
+      break;
+  }
+
   RefPtr<SharedMessagePortMessage> data = mMessages.ElementAt(0);
   mMessages.RemoveElementAt(0);
 
   RefPtr<PostMessageRunnable> runnable = new PostMessageRunnable(this, data);
 
   MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToCurrentThread(runnable)));
 
   mDispatchRunnable = new DispatchEventRunnable(this);
 
   MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToCurrentThread(mDispatchRunnable)));
 }
 
 void
 MessagePort::Close()
 {
+  CloseInternal(true /* aSoftly */);
+}
+
+void
+MessagePort::CloseForced()
+{
+  CloseInternal(false /* aSoftly */);
+}
+
+void
+MessagePort::CloseInternal(bool aSoftly)
+{
+  // If we have some messages to send but we don't want a 'soft' close, we have
+  // to flush them now.
+  if (!aSoftly) {
+    mMessages.Clear();
+  }
+
   if (mState == eStateUnshippedEntangled) {
     MOZ_ASSERT(mUnshippedEntangledPort);
 
     // This avoids loops.
     RefPtr<MessagePort> port = Move(mUnshippedEntangledPort);
     MOZ_ASSERT(mUnshippedEntangledPort == nullptr);
 
-    mState = eStateDisentangled;
-    port->Close();
+    mState = eStateDisentangledForClose;
+    port->CloseInternal(aSoftly);
 
     UpdateMustKeepAlive();
     return;
   }
 
   // Not entangled yet, we have to wait.
   if (mState == eStateEntangling) {
-    mNextStep = eNextStepClose;
+    mState = eStateEntanglingForClose;
+    return;
+  }
+
+  // Not entangled but already cloned or closed
+  if (mState == eStateEntanglingForDisentangle ||
+      mState == eStateEntanglingForClose) {
     return;
   }
 
   if (mState > eStateEntangled) {
     return;
   }
 
   // We don't care about stopping the sending of messages because from now all
   // the incoming messages will be ignored.
-  mState = eStateDisentangled;
+  mState = eStateDisentangledForClose;
 
   MOZ_ASSERT(mActor);
 
   mActor->SendClose();
   mActor->SetPort(nullptr);
   mActor = nullptr;
 
   UpdateMustKeepAlive();
@@ -571,18 +645,21 @@ MessagePort::SetOnmessage(EventHandlerNo
 
 // This method is called when the PMessagePortChild actor is entangled to
 // another actor. It receives a list of messages to be dispatch. It can be that
 // we were waiting for this entangling step in order to disentangle the port or
 // to close it.
 void
 MessagePort::Entangled(nsTArray<MessagePortMessage>& aMessages)
 {
-  MOZ_ASSERT(mState == eStateEntangling);
+  MOZ_ASSERT(mState == eStateEntangling ||
+             mState == eStateEntanglingForDisentangle ||
+             mState == eStateEntanglingForClose);
 
+  State oldState = mState;
   mState = eStateEntangled;
 
   // If we have pending messages, these have to be sent.
   if (!mMessagesForTheOtherPort.IsEmpty()) {
     nsTArray<MessagePortMessage> messages;
     SharedMessagePortMessage::FromSharedToMessagesChild(mActor,
                                                         mMessagesForTheOtherPort,
                                                         messages);
@@ -593,55 +670,59 @@ MessagePort::Entangled(nsTArray<MessageP
   // We must convert the messages into SharedMessagePortMessages to avoid leaks.
   FallibleTArray<RefPtr<SharedMessagePortMessage>> data;
   if (NS_WARN_IF(!SharedMessagePortMessage::FromMessagesToSharedChild(aMessages,
                                                                       data))) {
     // OOM, we cannot continue.
     return;
   }
 
-  if (mNextStep == eNextStepClose) {
-    Close();
+  // If the next step is to close the port, we do it ignoring the received
+  // messages.
+  if (oldState == eStateEntanglingForClose) {
+    CloseForced();
     return;
   }
 
   mMessages.AppendElements(data);
 
   // We were waiting for the entangling callback in order to disentangle this
   // port immediately after.
-  if (mNextStep == eNextStepDisentangle) {
+  if (oldState == eStateEntanglingForDisentangle) {
     StartDisentangling();
     return;
   }
 
-  MOZ_ASSERT(mNextStep == eNextStepNone);
   Dispatch();
 }
 
 void
 MessagePort::StartDisentangling()
 {
   MOZ_ASSERT(mActor);
   MOZ_ASSERT(mState == eStateEntangled);
 
   mState = eStateDisentangling;
-  mNextStep = eNextStepNone;
 
   // Sending this message we communicate to the parent actor that we don't want
   // to receive any new messages. It is possible that a message has been
   // already sent but not received yet. So we have to collect all of them and
   // we send them in the SendDispatch() request.
   mActor->SendStopSendingData();
 }
 
 void
 MessagePort::MessagesReceived(nsTArray<MessagePortMessage>& aMessages)
 {
-  MOZ_ASSERT(mState == eStateEntangled || mState == eStateDisentangling);
-  MOZ_ASSERT(mNextStep == eNextStepNone);
+  MOZ_ASSERT(mState == eStateEntangled ||
+             mState == eStateDisentangling ||
+             // This last step can happen only if Close() has been called
+             // manually. At this point SendClose() is sent but we can still
+             // receive something until the Closing request is processed.
+             mState == eStateDisentangledForClose);
   MOZ_ASSERT(mMessagesForTheOtherPort.IsEmpty());
 
   RemoveDocFromBFCache();
 
   FallibleTArray<RefPtr<SharedMessagePortMessage>> data;
   if (NS_WARN_IF(!SharedMessagePortMessage::FromMessagesToSharedChild(aMessages,
                                                                       data))) {
     // OOM, We cannot continue.
@@ -695,17 +776,18 @@ MessagePort::CloneAndDisentangle(Message
   aIdentifier.neutered() = true;
 
   if (mState > eStateEntangled) {
     return;
   }
 
   // We already have a 'next step'. We have to consider this port as already
   // cloned/closed/disentangled.
-  if (mNextStep != eNextStepNone) {
+  if (mState == eStateEntanglingForDisentangle ||
+      mState == eStateEntanglingForClose) {
     return;
   }
 
   aIdentifier.uuid() = mIdentifier->uuid();
   aIdentifier.destinationUuid() = mIdentifier->destinationUuid();
   aIdentifier.sequenceId() = mIdentifier->sequenceId() + 1;
   aIdentifier.neutered() = false;
 
@@ -725,37 +807,38 @@ MessagePort::CloneAndDisentangle(Message
       mState = eStateDisentangled;
       UpdateMustKeepAlive();
       return;
     }
 
     // Register this component to PBackground.
     ConnectToPBackground();
 
-    mNextStep = eNextStepDisentangle;
+    mState = eStateEntanglingForDisentangle;
     return;
   }
 
   // Not entangled yet, we have to wait.
-  if (mState < eStateEntangled) {
-    mNextStep = eNextStepDisentangle;
+  if (mState == eStateEntangling) {
+    mState = eStateEntanglingForDisentangle;
     return;
   }
 
+  MOZ_ASSERT(mState == eStateEntangled);
   StartDisentangling();
 }
 
 void
 MessagePort::Closed()
 {
-  if (mState == eStateDisentangled) {
+  if (mState >= eStateDisentangled) {
     return;
   }
 
-  mState = eStateDisentangled;
+  mState = eStateDisentangledForClose;
 
   if (mActor) {
     mActor->SetPort(nullptr);
     mActor = nullptr;
   }
 
   UpdateMustKeepAlive();
 }
@@ -784,33 +867,37 @@ MessagePort::ActorFailed()
 }
 
 void
 MessagePort::ActorCreated(mozilla::ipc::PBackgroundChild* aActor)
 {
   MOZ_ASSERT(aActor);
   MOZ_ASSERT(!mActor);
   MOZ_ASSERT(mIdentifier);
-  MOZ_ASSERT(mState == eStateEntangling);
+  MOZ_ASSERT(mState == eStateEntangling ||
+             mState == eStateEntanglingForDisentangle ||
+             mState == eStateEntanglingForClose);
 
   PMessagePortChild* actor =
     aActor->SendPMessagePortConstructor(mIdentifier->uuid(),
                                         mIdentifier->destinationUuid(),
                                         mIdentifier->sequenceId());
 
   mActor = static_cast<MessagePortChild*>(actor);
   MOZ_ASSERT(mActor);
 
   mActor->SetPort(this);
 }
 
 void
 MessagePort::UpdateMustKeepAlive()
 {
-  if (mState == eStateDisentangled && mIsKeptAlive) {
+  if (mState >= eStateDisentangled &&
+      mMessages.IsEmpty() &&
+      mIsKeptAlive) {
     mIsKeptAlive = false;
 
     if (mWorkerFeature) {
       WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
       MOZ_ASSERT(workerPrivate);
 
       workerPrivate->RemoveFeature(workerPrivate->GetJSContext(),
                                    mWorkerFeature);
@@ -854,17 +941,17 @@ MessagePort::Observe(nsISupports* aSubje
   nsCOMPtr<nsISupportsPRUint64> wrapper = do_QueryInterface(aSubject);
   NS_ENSURE_TRUE(wrapper, NS_ERROR_FAILURE);
 
   uint64_t innerID;
   nsresult rv = wrapper->GetData(&innerID);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (innerID == mInnerID) {
-    Close();
+    CloseForced();
   }
 
   return NS_OK;
 }
 
 void
 MessagePort::RemoveDocFromBFCache()
 {
--- a/dom/messagechannel/MessagePort.h
+++ b/dom/messagechannel/MessagePort.h
@@ -20,43 +20,46 @@ class nsPIDOMWindow;
 
 namespace mozilla {
 namespace dom {
 
 class DispatchEventRunnable;
 class MessagePortChild;
 class MessagePortIdentifier;
 class MessagePortMessage;
+class PostMessageRunnable;
 class SharedMessagePortMessage;
 
 namespace workers {
 class WorkerFeature;
 } // namespace workers
 
 class MessagePort final : public DOMEventTargetHelper
                         , public nsIIPCBackgroundChildCreateCallback
                         , public nsIObserver
 {
   friend class DispatchEventRunnable;
+  friend class PostMessageRunnable;
 
 public:
   NS_DECL_NSIIPCBACKGROUNDCHILDCREATECALLBACK
   NS_DECL_NSIOBSERVER
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(MessagePort,
                                            DOMEventTargetHelper)
 
   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);
 
+  // For IPC.
   static void
   ForceClose(const MessagePortIdentifier& aIdentifier);
 
   virtual JSObject*
   WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   void
   PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
@@ -72,16 +75,18 @@ public:
   void SetOnmessage(EventHandlerNonNull* aCallback);
 
   // Non WebIDL methods
 
   void UnshippedEntangle(MessagePort* aEntangledPort);
 
   void CloneAndDisentangle(MessagePortIdentifier& aIdentifier);
 
+  void CloseForced();
+
   // These methods are useful for MessagePortChild
 
   void Entangled(nsTArray<MessagePortMessage>& aMessages);
   void MessagesReceived(nsTArray<MessagePortMessage>& aMessages);
   void StopSendingDataConfirmed();
   void Closed();
 
 private:
@@ -91,21 +96,26 @@ private:
   enum State {
     // When a port is created by a MessageChannel it is entangled with the
     // other. They both run on the same thread, same event loop and the
     // messages are added to the queues without using PBackground actors.
     // When one of the port is shipped, the state is changed to
     // StateEntangling.
     eStateUnshippedEntangled,
 
-    // If the port is closed or cloned when we are in this state, we set the
-    // mNextStep. This 'next' operation will be done when entangled() message
-    // is received.
+    // If the port is closed or cloned when we are in this state, we go in one
+    // of the following 2 steps. EntanglingForClose or ForDisentangle.
     eStateEntangling,
 
+    // We are not fully entangled yet but are already disentangled.
+    eStateEntanglingForDisentangle,
+
+    // We are not fully entangled yet but are already closed.
+    eStateEntanglingForClose,
+
     // When entangled() is received we send all the messages in the
     // mMessagesForTheOtherPort to the actor and we change the state to
     // StateEntangled. At this point the port is entangled with the other. We
     // send and receive messages.
     // If the port queue is not enabled, the received messages are stored in
     // the mMessages.
     eStateEntangled,
 
@@ -116,33 +126,39 @@ private:
     // dispatched.
     eStateDisentangling,
 
     // When 'StopSendingDataConfirmed' is received, we can disentangle the port
     // calling SendDisentangle in the actor because we are 100% sure that we
     // don't receive any other message, so nothing will be lost.
     // Disentangling the port we send all the messages from the mMessages
     // though the actor.
-    eStateDisentangled
+    eStateDisentangled,
+
+    // We are here if Close() has been called. We are disentangled but we can
+    // still send pending messages.
+    eStateDisentangledForClose
   };
 
   void Initialize(const nsID& aUUID, const nsID& aDestinationUUID,
                   uint32_t aSequenceID, bool mNeutered, State aState,
                   ErrorResult& aRv);
 
   void ConnectToPBackground();
 
   // Dispatch events from the Message Queue using a nsRunnable.
   void Dispatch();
 
   void StartDisentangling();
   void Disentangle();
 
   void RemoveDocFromBFCache();
 
+  void CloseInternal(bool aSoftly);
+
   // This method is meant to keep alive the MessagePort when this object is
   // creating the actor and until the actor is entangled.
   // We release the object when the port is closed or disentangled.
   void UpdateMustKeepAlive();
 
   bool IsCertainlyAliveForCC() const override
   {
     return mIsKeptAlive;
@@ -160,24 +176,16 @@ private:
   nsTArray<RefPtr<SharedMessagePortMessage>> mMessagesForTheOtherPort;
 
   nsAutoPtr<MessagePortIdentifier> mIdentifier;
 
   uint64_t mInnerID;
 
   State mState;
 
-  // This 'nextStep' is used when we are waiting to be entangled but the
-  // content has called Clone() or Close().
-  enum {
-    eNextStepNone,
-    eNextStepDisentangle,
-    eNextStepClose
-  } mNextStep;
-
   bool mMessageQueueEnabled;
 
   bool mIsKeptAlive;
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/messagechannel/MessagePortService.cpp
+++ b/dom/messagechannel/MessagePortService.cpp
@@ -31,16 +31,19 @@ AssertIsInMainProcess()
 
 class MessagePortService::MessagePortServiceData final
 {
 public:
   explicit MessagePortServiceData(const nsID& aDestinationUUID)
     : mDestinationUUID(aDestinationUUID)
     , mSequenceID(1)
     , mParent(nullptr)
+    // By default we don't know the next parent.
+    , mWaitingForNewParent(true)
+    , mNextStepCloseAll(false)
   {
     MOZ_COUNT_CTOR(MessagePortServiceData);
   }
 
   MessagePortServiceData(const MessagePortServiceData& aOther) = delete;
   MessagePortServiceData& operator=(const MessagePortServiceData&) = delete;
 
   ~MessagePortServiceData()
@@ -57,16 +60,19 @@ public:
   {
     uint32_t mSequenceID;
     // MessagePortParent keeps the service alive, and we don't want a cycle.
     MessagePortParent* mParent;
   };
 
   FallibleTArray<NextParent> mNextParents;
   FallibleTArray<RefPtr<SharedMessagePortMessage>> mMessages;
+
+  bool mWaitingForNewParent;
+  bool mNextStepCloseAll;
 };
 
 /* static */ MessagePortService*
 MessagePortService::Get()
 {
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
 
@@ -108,48 +114,67 @@ MessagePortService::RequestEntangling(Me
 
     data = new MessagePortServiceData(aDestinationUUID);
     mPorts.Put(aParent->ID(), data);
   }
 
   // This is a security check.
   if (!data->mDestinationUUID.Equals(aDestinationUUID)) {
     MOZ_ASSERT(false, "DestinationUUIDs do not match!");
+    CloseAll(aParent->ID());
     return false;
   }
 
   if (aSequenceID < data->mSequenceID) {
     MOZ_ASSERT(false, "Invalid sequence ID!");
+    CloseAll(aParent->ID());
     return false;
   }
 
   if (aSequenceID == data->mSequenceID) {
     if (data->mParent) {
       MOZ_ASSERT(false, "Two ports cannot have the same sequenceID.");
+      CloseAll(aParent->ID());
       return false;
     }
 
     // We activate this port, sending all the messages.
     data->mParent = aParent;
+    data->mWaitingForNewParent = false;
     FallibleTArray<MessagePortMessage> array;
     if (!SharedMessagePortMessage::FromSharedToMessagesParent(aParent,
                                                               data->mMessages,
                                                               array)) {
+      CloseAll(aParent->ID());
       return false;
     }
 
     data->mMessages.Clear();
-    return aParent->Entangled(array);
+
+    // We can entangle the port.
+    if (!aParent->Entangled(array)) {
+      CloseAll(aParent->ID());
+      return false;
+    }
+
+    // If we were waiting for this parent in order to close this channel, this
+    // is the time to do it.
+    if (data->mNextStepCloseAll) {
+      CloseAll(aParent->ID());
+    }
+
+    return true;
   }
 
   // This new parent will be the next one when a Disentangle request is
   // received from the current parent.
   MessagePortServiceData::NextParent* nextParent =
     data->mNextParents.AppendElement(mozilla::fallible);
   if (!nextParent) {
+    CloseAll(aParent->ID());
     return false;
   }
 
   nextParent->mSequenceID = aSequenceID;
   nextParent->mParent = aParent;
 
   return true;
 }
@@ -188,16 +213,17 @@ MessagePortService::DisentanglePort(
       nextParent = data->mNextParents[index].mParent;
       break;
     }
   }
 
   // We didn't find the parent.
   if (!nextParent) {
     data->mMessages.SwapElements(aMessages);
+    data->mWaitingForNewParent = true;
     data->mParent = nullptr;
     return true;
   }
 
   data->mParent = nextParent;
   data->mNextParents.RemoveElementAt(index);
 
   FallibleTArray<MessagePortMessage> array;
@@ -262,16 +288,30 @@ MessagePortService::CloseAll(const nsID&
     data->mParent->Close();
   }
 
   for (const MessagePortServiceData::NextParent& parent : data->mNextParents) {
     parent.mParent->CloseAndDelete();
   }
 
   nsID destinationUUID = data->mDestinationUUID;
+
+  // If we have informations about the other port and that port has some
+  // pending messages to deliver but the parent has not processed them yet,
+  // because its entangling request didn't arrive yet), we cannot close this
+  // channel.
+  MessagePortServiceData* destinationData;
+  if (mPorts.Get(destinationUUID, &destinationData) &&
+      !destinationData->mMessages.IsEmpty() &&
+      destinationData->mWaitingForNewParent) {
+    MOZ_ASSERT(!destinationData->mNextStepCloseAll);
+    destinationData->mNextStepCloseAll = true;
+    return;
+  }
+
   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;
--- a/dom/messagechannel/tests/mochitest.ini
+++ b/dom/messagechannel/tests/mochitest.ini
@@ -19,8 +19,10 @@ support-files =
 [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]
 [test_messageChannel_bug1178076.html]
+[test_messageChannel_bug1224825.html]
+[test_messageChannel_worker_forceClose.html]
new file mode 100644
--- /dev/null
+++ b/dom/messagechannel/tests/test_messageChannel_bug1224825.html
@@ -0,0 +1,94 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1224825
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1224825</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=1224825">Mozilla Bug 1224825</a>
+<div id="content"></div>
+<pre id="test">
+</pre>
+  <script type="application/javascript">
+
+var MAX = 100;
+
+function test_fullDeliveredMessages() {
+  var worker = new Worker('data:javascript,onmessage = function(e) { e.ports[0].onmessage = function(evt) { postMessage(evt.data);}}');
+
+  var count = 0;
+  worker.onmessage = function(e) {
+    is(e.data, count, "Correct value expected!");
+    ok(count < MAX,"No count > MAX messages!");
+    if (++count == MAX) {
+
+      SimpleTest.requestFlakyTimeout("Testing an event not happening");
+      setTimeout(function() {
+        runTests();
+      }, 200);
+
+      info("All the messages correctly received");
+    }
+  }
+
+  var mc = new MessageChannel();
+  worker.postMessage(42, [mc.port2]);
+
+  for (var i = 0; i < MAX; ++i) {
+    mc.port1.postMessage(i);
+  }
+
+  mc.port1.close();
+
+  for (var i = 0; i < MAX * 2; ++i) {
+    mc.port1.postMessage(i);
+  }
+}
+
+function test_closeInBetween() {
+  var mc = new MessageChannel();
+
+  for (var i = 0; i < MAX; ++i) {
+    mc.port1.postMessage(i);
+  }
+
+  mc.port1.onmessage = function(e) {
+    ok (e.data < MAX/2, "Correct message received from port1:" + e.data);
+  }
+
+  mc.port2.onmessage = function(e) {
+    ok (e.data < MAX, "Correct message received from port2:" + e.data);
+    if (e.data == MAX/2) {
+      mc.port2.close();
+    }
+
+    mc.port2.postMessage(e.data);
+
+    if (e.data == MAX - 1) {
+      runTests();
+    }
+  }
+}
+
+var tests = [ test_fullDeliveredMessages, test_closeInBetween ];
+
+function runTests() {
+  if (!tests.length) {
+    SimpleTest.finish();
+    return;
+  }
+
+  var test = tests.shift();
+  test();
+}
+
+SimpleTest.waitForExplicitFinish();
+runTests();
+  </script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/messagechannel/tests/test_messageChannel_worker_forceClose.html
@@ -0,0 +1,27 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test for forcing the closing of the port in workers</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<div id="content"></div>
+<pre id="test">
+</pre>
+  <script type="application/javascript">
+
+  var worker = new Worker('data:javascript,onmessage = function(e) { "doing nothing with this port"; }');
+
+  var mc = new MessageChannel();
+  worker.postMessage(42, [mc.port2]);
+
+  for (var i = 0; i < 10; ++i) {
+    mc.port1.postMessage(i);
+  }
+
+  ok(true, "All the messages are sent! We should shutdown correctly.");
+  </script>
+</body>
+</html>