From 554272fa7f6c4dc78b74a505a1bf68f8d4181d6c Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Mon, 21 Sep 2015 16:54:40 +0200 Subject: [PATCH] Bug 1072313 - Never call TextureClient::KeepUntilFullDeallocation off the ipdl thread. r=mattwoodrow --- gfx/layers/client/TextureClient.cpp | 1 + gfx/layers/client/TextureClient.h | 11 +++++++- gfx/layers/d3d11/TextureD3D11.cpp | 26 +++++++++++++------ gfx/layers/d3d11/TextureD3D11.h | 4 +++ gfx/layers/d3d9/TextureD3D9.cpp | 7 ++++- gfx/layers/d3d9/TextureD3D9.h | 2 ++ gfx/layers/ipc/ImageBridgeChild.cpp | 2 +- .../opengl/MacIOSurfaceTextureClientOGL.cpp | 5 ++++ .../opengl/MacIOSurfaceTextureClientOGL.h | 2 ++ 9 files changed, 49 insertions(+), 11 deletions(-) diff --git a/gfx/layers/client/TextureClient.cpp b/gfx/layers/client/TextureClient.cpp index ce847d311b22..23e899c353b0 100644 --- a/gfx/layers/client/TextureClient.cpp +++ b/gfx/layers/client/TextureClient.cpp @@ -557,6 +557,7 @@ TextureClient::KeepUntilFullDeallocation(UniquePtr aKeep, bool aMainT void TextureClient::ForceRemove(bool sync) { + FinalizeOnIPDLThread(); if (mValid && mActor) { if (sync || GetFlags() & TextureFlags::DEALLOCATE_CLIENT) { MOZ_PERFORMANCE_WARNING("gfx", "TextureClient/Host pair requires synchronous deallocation"); diff --git a/gfx/layers/client/TextureClient.h b/gfx/layers/client/TextureClient.h index 1337cbfed7f6..373b5a307486 100644 --- a/gfx/layers/client/TextureClient.h +++ b/gfx/layers/client/TextureClient.h @@ -408,6 +408,8 @@ public: * help avoid race conditions in some cases. * It's a temporary hack to ensure that DXGI textures don't get destroyed * between serialization and deserialization. + * + * This must not be called off the texture's IPDL thread. */ void KeepUntilFullDeallocation(UniquePtr aKeep, bool aMainThreadOnly = false); @@ -504,13 +506,20 @@ private: static void TextureClientRecycleCallback(TextureClient* aClient, void* aClosure); /** - * Called once, just before the destructor. + * Called once, during the destruction of the Texture, on the thread in which + * texture's reference count reaches 0 (could be any thread). * * Here goes the shut-down code that uses virtual methods. * Must only be called by Release(). */ B2G_ACL_EXPORT void Finalize(); + /** + * Called once during the destruction of the texture on the IPDL thread, if + * the texture is shared on the compositor (otherwise it is not called at all). + */ + virtual void FinalizeOnIPDLThread() {} + friend class AtomicRefCountedWithFinalize; protected: diff --git a/gfx/layers/d3d11/TextureD3D11.cpp b/gfx/layers/d3d11/TextureD3D11.cpp index bf0f945da777..51c8513316ec 100644 --- a/gfx/layers/d3d11/TextureD3D11.cpp +++ b/gfx/layers/d3d11/TextureD3D11.cpp @@ -198,13 +198,6 @@ TextureClientD3D11::TextureClientD3D11(ISurfaceAllocator* aAllocator, TextureClientD3D11::~TextureClientD3D11() { - if (mActor) { - if (mTexture) { - KeepUntilFullDeallocation(MakeUnique>(mTexture)); - } else if (mTexture10) { - KeepUntilFullDeallocation(MakeUnique>(mTexture10)); - } - } #ifdef DEBUG // An Azure DrawTarget needs to be locked when it gets nullptr'ed as this is // when it calls EndDraw. This EndDraw should not execute anything so it @@ -228,6 +221,18 @@ TextureClientD3D11::~TextureClientD3D11() #endif } +void +TextureClientD3D11::FinalizeOnIPDLThread() +{ + if (mActor) { + if (mTexture) { + KeepUntilFullDeallocation(MakeUnique>(mTexture)); + } else if (mTexture10) { + KeepUntilFullDeallocation(MakeUnique>(mTexture10)); + } + } +} + // static already_AddRefed TextureClientD3D11::Create(ISurfaceAllocator* aAllocator, @@ -656,11 +661,16 @@ protected: }; DXGIYCbCrTextureClient::~DXGIYCbCrTextureClient() +{ + MOZ_COUNT_DTOR(DXGIYCbCrTextureClient); +} + +void +DXGIYCbCrTextureClient::FinalizeOnIPDLThread() { if (mHoldRefs[0] && mActor) { KeepUntilFullDeallocation(MakeUnique(mHoldRefs), true); } - MOZ_COUNT_DTOR(DXGIYCbCrTextureClient); } // static diff --git a/gfx/layers/d3d11/TextureD3D11.h b/gfx/layers/d3d11/TextureD3D11.h index c3d1ad8c002d..b19877ce49b2 100644 --- a/gfx/layers/d3d11/TextureD3D11.h +++ b/gfx/layers/d3d11/TextureD3D11.h @@ -88,6 +88,8 @@ public: protected: bool AllocateD3D11Surface(ID3D11Device* aDevice, const gfx::IntSize& aSize); + virtual void FinalizeOnIPDLThread() override; + gfx::IntSize mSize; RefPtr mTexture10; RefPtr mTexture; @@ -157,6 +159,8 @@ public: CreateSimilar(TextureFlags, TextureAllocationFlags) const override{ return nullptr; } private: + virtual void FinalizeOnIPDLThread() override; + RefPtr mHoldRefs[3]; HANDLE mHandles[3]; gfx::IntSize mSize; diff --git a/gfx/layers/d3d9/TextureD3D9.cpp b/gfx/layers/d3d9/TextureD3D9.cpp index 2f46658cdb2c..17ea1e48c35b 100644 --- a/gfx/layers/d3d9/TextureD3D9.cpp +++ b/gfx/layers/d3d9/TextureD3D9.cpp @@ -660,6 +660,12 @@ SharedTextureClientD3D9::SharedTextureClientD3D9(ISurfaceAllocator* aAllocator, } SharedTextureClientD3D9::~SharedTextureClientD3D9() +{ + MOZ_COUNT_DTOR(SharedTextureClientD3D9); +} + +void +SharedTextureClientD3D9::FinalizeOnIPDLThread() { if (mTexture && mActor) { KeepUntilFullDeallocation(MakeUnique>(mTexture)); @@ -667,7 +673,6 @@ SharedTextureClientD3D9::~SharedTextureClientD3D9() if (mTexture) { gfxWindowsPlatform::sD3D9SharedTextureUsed -= mDesc.Width * mDesc.Height * 4; } - MOZ_COUNT_DTOR(SharedTextureClientD3D9); } // static diff --git a/gfx/layers/d3d9/TextureD3D9.h b/gfx/layers/d3d9/TextureD3D9.h index 27ff3093be66..5708775af9d0 100644 --- a/gfx/layers/d3d9/TextureD3D9.h +++ b/gfx/layers/d3d9/TextureD3D9.h @@ -285,6 +285,8 @@ public: } private: + virtual void FinalizeOnIPDLThread() override; + RefPtr mDevice; RefPtr mTexture; gfx::SurfaceFormat mFormat; diff --git a/gfx/layers/ipc/ImageBridgeChild.cpp b/gfx/layers/ipc/ImageBridgeChild.cpp index 301c6b578479..06cb4ca773dc 100644 --- a/gfx/layers/ipc/ImageBridgeChild.cpp +++ b/gfx/layers/ipc/ImageBridgeChild.cpp @@ -188,7 +188,7 @@ static void ImageBridgeShutdownStep1(ReentrantMonitor *aBarrier, bool *aDone) InfallibleTArray textures; sImageBridgeChildSingleton->ManagedPTextureChild(textures); for (int i = textures.Length() - 1; i >= 0; --i) { - TextureClient* client = TextureClient::AsTextureClient(textures[i]); + RefPtr client = TextureClient::AsTextureClient(textures[i]); if (client) { client->ForceRemove(); } diff --git a/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.cpp b/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.cpp index 159b41291fc7..fbefd06c8f65 100644 --- a/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.cpp +++ b/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.cpp @@ -16,6 +16,11 @@ MacIOSurfaceTextureClientOGL::MacIOSurfaceTextureClientOGL(ISurfaceAllocator* aA {} MacIOSurfaceTextureClientOGL::~MacIOSurfaceTextureClientOGL() +{ +} + +void +MacIOSurfaceTextureClientOGL::FinalizeOnIPDLThread() { if (mActor && mSurface) { KeepUntilFullDeallocation(MakeUnique>(mSurface)); diff --git a/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.h b/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.h index 5a0c03825d36..7548beeafdc7 100644 --- a/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.h +++ b/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.h @@ -50,6 +50,8 @@ public: CreateSimilar(TextureFlags, TextureAllocationFlags) const override { return nullptr; } protected: + virtual void FinalizeOnIPDLThread() override; + RefPtr mSurface; bool mIsLocked; };