From 0502aa5138232e8c37227a876f555a0d2ec4e20f Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Tue, 23 Jun 2015 15:50:00 -0700 Subject: [PATCH] Bug 1176034 - MessagePort should force a close() if the structured clone algorithm fails, r=bent --- dom/base/PostMessageEvent.cpp | 8 ++- dom/messagechannel/MessagePort.cpp | 51 +++++++++++++++++++ dom/messagechannel/MessagePort.h | 3 ++ dom/messagechannel/MessagePortParent.cpp | 14 +++++ dom/messagechannel/MessagePortParent.h | 4 ++ dom/messagechannel/MessagePortService.cpp | 50 ++++++++++++++++++ dom/messagechannel/MessagePortService.h | 5 ++ dom/messagechannel/MessagePortUtils.cpp | 18 ++++++- dom/messagechannel/tests/mochitest.ini | 1 + .../tests/test_messageChannel_forceClose.html | 36 +++++++++++++ dom/workers/WorkerPrivate.cpp | 9 +++- ipc/glue/BackgroundParentImpl.cpp | 11 ++++ ipc/glue/BackgroundParentImpl.h | 5 ++ ipc/glue/PBackground.ipdl | 2 + 14 files changed, 213 insertions(+), 4 deletions(-) create mode 100644 dom/messagechannel/tests/test_messageChannel_forceClose.html diff --git a/dom/base/PostMessageEvent.cpp b/dom/base/PostMessageEvent.cpp index 534d389cda1..f375a672a72 100644 --- a/dom/base/PostMessageEvent.cpp +++ b/dom/base/PostMessageEvent.cpp @@ -238,7 +238,13 @@ PostMessageEvent::FreeTransferStructuredClone(uint32_t aTag, uint64_t aExtraData, void* aClosure) { - // Nothing to do. + if (aTag == SCTAG_DOM_MAP_MESSAGEPORT) { + MOZ_ASSERT(aClosure); + MOZ_ASSERT(!aContent); + + StructuredCloneInfo* scInfo = static_cast(aClosure); + MessagePort::ForceClose(scInfo->event->GetPortIdentifier(aExtraData)); + } } PostMessageEvent::PostMessageEvent(nsGlobalWindow* aSource, diff --git a/dom/messagechannel/MessagePort.cpp b/dom/messagechannel/MessagePort.cpp index 6a328bb969e..fa36218a744 100644 --- a/dom/messagechannel/MessagePort.cpp +++ b/dom/messagechannel/MessagePort.cpp @@ -21,6 +21,7 @@ #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" @@ -249,6 +250,50 @@ private: } }; +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 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) @@ -863,5 +908,11 @@ MessagePort::RemoveDocFromBFCache() bfCacheEntry->RemoveFromBFCacheSync(); } +/* static */ void +MessagePort::ForceClose(const MessagePortIdentifier& aIdentifier) +{ + ForceCloseHelper::ForceClose(aIdentifier); +} + } // namespace dom } // namespace mozilla diff --git a/dom/messagechannel/MessagePort.h b/dom/messagechannel/MessagePort.h index 63081a08332..ede640ba6ae 100644 --- a/dom/messagechannel/MessagePort.h +++ b/dom/messagechannel/MessagePort.h @@ -86,6 +86,9 @@ public: Create(nsPIDOMWindow* aWindow, const MessagePortIdentifier& aIdentifier, ErrorResult& aRv); + static void + ForceClose(const MessagePortIdentifier& aIdentifier); + virtual JSObject* WrapObject(JSContext* aCx, JS::Handle aGivenProto) override; diff --git a/dom/messagechannel/MessagePortParent.cpp b/dom/messagechannel/MessagePortParent.cpp index 25dadafdab3..917606bf09d 100644 --- a/dom/messagechannel/MessagePortParent.cpp +++ b/dom/messagechannel/MessagePortParent.cpp @@ -159,5 +159,19 @@ MessagePortParent::Close() 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 diff --git a/dom/messagechannel/MessagePortParent.h b/dom/messagechannel/MessagePortParent.h index 46dbb19ff0c..1dcfb53e79f 100644 --- a/dom/messagechannel/MessagePortParent.h +++ b/dom/messagechannel/MessagePortParent.h @@ -36,6 +36,10 @@ public: return mUUID; } + static bool ForceClose(const nsID& aUUID, + const nsID& aDestinationUUID, + const uint32_t& aSequenceID); + private: virtual bool RecvPostMessages(nsTArray&& aMessages) override; diff --git a/dom/messagechannel/MessagePortService.cpp b/dom/messagechannel/MessagePortService.cpp index 0d75e6ba195..afd5ab98af2 100644 --- a/dom/messagechannel/MessagePortService.cpp +++ b/dom/messagechannel/MessagePortService.cpp @@ -6,16 +6,27 @@ #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 gInstance; + +void +AssertIsInMainProcess() +{ + MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default); +} + } // anonymous namespace class MessagePortService::MessagePortServiceData final @@ -53,9 +64,21 @@ public: FallibleTArray> mMessages; }; +/* static */ MessagePortService* +MessagePortService::Get() +{ + AssertIsInMainProcess(); + AssertIsOnBackgroundThread(); + + return gInstance; +} + /* static */ MessagePortService* MessagePortService::GetOrCreate() { + AssertIsInMainProcess(); + AssertIsOnBackgroundThread(); + if (!gInstance) { gInstance = new MessagePortService(); } @@ -248,6 +271,12 @@ MessagePortService::CloseAll(const nsID& 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(&aUUID)); #endif @@ -324,5 +353,26 @@ MessagePortService::ParentDestroy(MessagePortParent* aParent) 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 diff --git a/dom/messagechannel/MessagePortService.h b/dom/messagechannel/MessagePortService.h index 437cf2393c1..faae81fcbc3 100644 --- a/dom/messagechannel/MessagePortService.h +++ b/dom/messagechannel/MessagePortService.h @@ -20,6 +20,7 @@ class MessagePortService final public: NS_INLINE_DECL_REFCOUNTING(MessagePortService) + static MessagePortService* Get(); static MessagePortService* GetOrCreate(); bool RequestEntangling(MessagePortParent* aParent, @@ -38,6 +39,10 @@ public: void ParentDestroy(MessagePortParent* aParent); + bool ForceClose(const nsID& aUUID, + const nsID& aDestinationUUID, + const uint32_t& aSequenceID); + private: ~MessagePortService() {} diff --git a/dom/messagechannel/MessagePortUtils.cpp b/dom/messagechannel/MessagePortUtils.cpp index 06b330b7f42..ccd3b13043e 100644 --- a/dom/messagechannel/MessagePortUtils.cpp +++ b/dom/messagechannel/MessagePortUtils.cpp @@ -141,7 +141,7 @@ ReadTransfer(JSContext* aCx, JSStructuredCloneReader* aReader, ErrorResult rv; nsRefPtr port = MessagePort::Create(closure->mWindow, - closure->mClosure.mMessagePortIdentifiers[(uint32_t)aExtraData], + closure->mClosure.mMessagePortIdentifiers[aExtraData], rv); if (NS_WARN_IF(rv.Failed())) { return false; @@ -195,13 +195,27 @@ WriteTransfer(JSContext* aCx, JS::Handle aObj, void* aClosure, return true; } +void +FreeTransfer(uint32_t aTag, JS::TransferableOwnership aOwnership, + void* aContent, uint64_t aExtraData, void* aClosure) +{ + MOZ_ASSERT(aClosure); + auto* closure = static_cast(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 diff --git a/dom/messagechannel/tests/mochitest.ini b/dom/messagechannel/tests/mochitest.ini index fc5cecd1304..8633b3cdc9f 100644 --- a/dom/messagechannel/tests/mochitest.ini +++ b/dom/messagechannel/tests/mochitest.ini @@ -23,3 +23,4 @@ support-files = [test_messageChannel_sharedWorker.html] [test_messageChannel_sharedWorker2.html] [test_messageChannel_any.html] +[test_messageChannel_forceClose.html] diff --git a/dom/messagechannel/tests/test_messageChannel_forceClose.html b/dom/messagechannel/tests/test_messageChannel_forceClose.html new file mode 100644 index 00000000000..dae2292ab8d --- /dev/null +++ b/dom/messagechannel/tests/test_messageChannel_forceClose.html @@ -0,0 +1,36 @@ + + + + + + Test for Bug 1176034 - start/close + + + + +Mozilla Bug 1176034 +
+
+
+ + + diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 3ad44e905d3..43eac9e7b54 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -763,7 +763,14 @@ struct WorkerStructuredCloneCallbacks 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(aClosure); + + MOZ_ASSERT(aExtraData < closure->mMessagePortIdentifiers.Length()); + dom::MessagePort::ForceClose(closure->mMessagePortIdentifiers[aExtraData]); + } } }; diff --git a/ipc/glue/BackgroundParentImpl.cpp b/ipc/glue/BackgroundParentImpl.cpp index f29ed4fc663..8956ab5d509 100644 --- a/ipc/glue/BackgroundParentImpl.cpp +++ b/ipc/glue/BackgroundParentImpl.cpp @@ -588,6 +588,17 @@ BackgroundParentImpl::DeallocPMessagePortParent(PMessagePortParent* 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 diff --git a/ipc/glue/BackgroundParentImpl.h b/ipc/glue/BackgroundParentImpl.h index 6dd54f75f26..777303f8fee 100644 --- a/ipc/glue/BackgroundParentImpl.h +++ b/ipc/glue/BackgroundParentImpl.h @@ -137,6 +137,11 @@ protected: virtual bool DeallocPMessagePortParent(PMessagePortParent* aActor) override; + + virtual bool + RecvMessagePortForceClose(const nsID& aUUID, + const nsID& aDestinationUUID, + const uint32_t& aSequenceID) override; }; } // namespace ipc diff --git a/ipc/glue/PBackground.ipdl b/ipc/glue/PBackground.ipdl index 6b09ed079cb..705b094ed06 100644 --- a/ipc/glue/PBackground.ipdl +++ b/ipc/glue/PBackground.ipdl @@ -60,6 +60,8 @@ parent: PMessagePort(nsID uuid, nsID destinationUuid, uint32_t sequenceId); + MessagePortForceClose(nsID uuid, nsID destinationUuid, uint32_t sequenceId); + child: PCache(); PCacheStreamControl();