Bug 1534749 - Handle shutdown races between the compositor thread and IPDL object setup. r=rhunt

When the compositor thread has begun shutdown, it will spin the event
loop for the main thread until the last CompositorThreadHolder reference
has been released. While spinning, new IPDL objects may be attempted to
be created which depend on the compositor thread; we should check to
ensure the compositor thread is still around before proceeding with
creation. These objects include CompositorManagerParent,
ImageBridgeParent, and VRManagerParent. Additionally there is a very
similar bug between the vsync thread and VsyncBridgeChild.

Differential Revision: https://phabricator.services.mozilla.com/D23308
This commit is contained in:
Andrew Osmond 2019-03-12 15:08:01 -04:00
parent d91b7a630f
commit 5756f575b9
8 changed files with 32 additions and 12 deletions

View File

@ -804,8 +804,8 @@ bool GPUProcessManager::CreateContentCompositorManager(
if (mGPUChild) {
mGPUChild->SendNewContentCompositorManager(std::move(parentPipe));
} else {
CompositorManagerParent::Create(std::move(parentPipe));
} else if (!CompositorManagerParent::Create(std::move(parentPipe))) {
return false;
}
*aOutEndpoint = std::move(childPipe);

View File

@ -12,7 +12,7 @@ namespace gfx {
VsyncBridgeChild::VsyncBridgeChild(RefPtr<VsyncIOThreadHolder> aThread,
const uint64_t& aProcessToken)
: mThread(aThread), mLoop(nullptr), mProcessToken(aProcessToken) {}
: mThread(aThread), mProcessToken(aProcessToken) {}
VsyncBridgeChild::~VsyncBridgeChild() {}
@ -39,8 +39,6 @@ void VsyncBridgeChild::Open(Endpoint<PVsyncBridgeChild>&& aEndpoint) {
return;
}
mLoop = MessageLoop::current();
// Last reference is freed in DeallocPVsyncBridgeChild.
AddRef();
}
@ -66,7 +64,7 @@ class NotifyVsyncTask : public Runnable {
};
bool VsyncBridgeChild::IsOnVsyncIOThread() const {
return MessageLoop::current() == mLoop;
return mThread->IsOnCurrentThread();
}
void VsyncBridgeChild::NotifyVsync(const VsyncEvent& aVsync,
@ -75,7 +73,7 @@ void VsyncBridgeChild::NotifyVsync(const VsyncEvent& aVsync,
MOZ_ASSERT(!IsOnVsyncIOThread());
RefPtr<NotifyVsyncTask> task = new NotifyVsyncTask(this, aVsync, aLayersId);
mLoop->PostTask(task.forget());
mThread->Dispatch(task.forget());
}
void VsyncBridgeChild::NotifyVsyncImpl(const VsyncEvent& aVsync,
@ -91,8 +89,8 @@ void VsyncBridgeChild::NotifyVsyncImpl(const VsyncEvent& aVsync,
void VsyncBridgeChild::Close() {
if (!IsOnVsyncIOThread()) {
mLoop->PostTask(NewRunnableMethod("gfx::VsyncBridgeChild::Close", this,
&VsyncBridgeChild::Close));
mThread->Dispatch(NewRunnableMethod("gfx::VsyncBridgeChild::Close", this,
&VsyncBridgeChild::Close));
return;
}

View File

@ -47,7 +47,6 @@ class VsyncBridgeChild final : public PVsyncBridgeChild {
private:
RefPtr<VsyncIOThreadHolder> mThread;
MessageLoop* mLoop;
uint64_t mProcessToken;
};

View File

@ -23,6 +23,14 @@ class VsyncIOThreadHolder final {
RefPtr<nsIThread> GetThread() const;
bool IsOnCurrentThread() const {
return mThread->IsOnCurrentThread();
}
void Dispatch(already_AddRefed<nsIRunnable> task) {
mThread->Dispatch(std::move(task), NS_DISPATCH_NORMAL);
}
private:
~VsyncIOThreadHolder();

View File

@ -49,7 +49,7 @@ CompositorManagerParent::CreateSameProcess() {
}
/* static */
void CompositorManagerParent::Create(
bool CompositorManagerParent::Create(
Endpoint<PCompositorManagerParent>&& aEndpoint) {
MOZ_ASSERT(NS_IsMainThread());
@ -57,6 +57,10 @@ void CompositorManagerParent::Create(
// (or UI process if it subsumbed the GPU process).
MOZ_ASSERT(aEndpoint.OtherPid() != base::GetCurrentProcId());
if (!CompositorThreadHolder::IsActive()) {
return false;
}
RefPtr<CompositorManagerParent> bridge = new CompositorManagerParent();
RefPtr<Runnable> runnable =
@ -64,6 +68,7 @@ void CompositorManagerParent::Create(
"CompositorManagerParent::Bind", bridge,
&CompositorManagerParent::Bind, std::move(aEndpoint));
CompositorThreadHolder::Loop()->PostTask(runnable.forget());
return true;
}
/* static */

View File

@ -30,7 +30,7 @@ class CompositorManagerParent final : public PCompositorManagerParent {
public:
static already_AddRefed<CompositorManagerParent> CreateSameProcess();
static void Create(Endpoint<PCompositorManagerParent>&& aEndpoint);
static bool Create(Endpoint<PCompositorManagerParent>&& aEndpoint);
static void Shutdown();
static already_AddRefed<CompositorBridgeParent>

View File

@ -91,6 +91,10 @@ bool ImageBridgeParent::CreateForGPUProcess(
MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_GPU);
MessageLoop* loop = CompositorThreadHolder::Loop();
if (!loop) {
return false;
}
RefPtr<ImageBridgeParent> parent =
new ImageBridgeParent(loop, aEndpoint.OtherPid());
@ -211,6 +215,9 @@ mozilla::ipc::IPCResult ImageBridgeParent::RecvUpdate(
bool ImageBridgeParent::CreateForContent(
Endpoint<PImageBridgeParent>&& aEndpoint) {
MessageLoop* loop = CompositorThreadHolder::Loop();
if (!loop) {
return false;
}
RefPtr<ImageBridgeParent> bridge =
new ImageBridgeParent(loop, aEndpoint.OtherPid());

View File

@ -74,6 +74,9 @@ void VRManagerParent::UnregisterFromManager() {
/* static */
bool VRManagerParent::CreateForContent(Endpoint<PVRManagerParent>&& aEndpoint) {
MessageLoop* loop = CompositorThreadHolder::Loop();
if (!loop) {
return false;
}
RefPtr<VRManagerParent> vmp = new VRManagerParent(aEndpoint.OtherPid(), true);
loop->PostTask(NewRunnableMethod<Endpoint<PVRManagerParent>&&>(