Bug 1015718 - Keep the client-side D3D11 texture alive longer so that its destruction does not race with its deserialization. r=bas, a=sledru
authorNicolas Silva <nsilva@mozilla.com>
Tue, 22 Jul 2014 17:49:05 +0200
changeset 217267 c3a6a7b045053c12c097372247f02af1a09d9af9
parent 217266 7fc1cdf6978ea065ac11406e05f00b48326cf678
child 217268 1e0b3a5dcde78c99801b44cf776a93d3cb0a6e72
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbas, sledru
bugs1015718
milestone33.0a2
Bug 1015718 - Keep the client-side D3D11 texture alive longer so that its destruction does not race with its deserialization. r=bas, a=sledru Deserialize the DXGI handle when constructing the TextureHost rather than when locking it. r=nical Don't crash creating a TextureHost if there is no device or if deserialization failed. fixing test failure
gfx/layers/client/TextureClient.cpp
gfx/layers/client/TextureClient.h
gfx/layers/d3d11/TextureD3D11.cpp
gfx/layers/d3d11/TextureD3D11.h
--- a/gfx/layers/client/TextureClient.cpp
+++ b/gfx/layers/client/TextureClient.cpp
@@ -76,16 +76,17 @@ class TextureChild MOZ_FINAL : public PT
 {
   ~TextureChild() {}
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(TextureChild)
 
   TextureChild()
   : mForwarder(nullptr)
   , mTextureClient(nullptr)
+  , mKeep(nullptr)
   , mIPCOpen(false)
   {
   }
 
   bool Recv__delete__() MOZ_OVERRIDE;
 
   bool RecvCompositorRecycle()
   {
@@ -124,16 +125,17 @@ private:
     MOZ_ASSERT(mIPCOpen == true);
     mIPCOpen = false;
     Release();
   }
 
   RefPtr<CompositableForwarder> mForwarder;
   RefPtr<TextureClient> mWaitForRecycle;
   TextureClient* mTextureClient;
+  KeepAlive* mKeep;
   bool mIPCOpen;
 
   friend class TextureClient;
 };
 
 bool
 TextureChild::Recv__delete__()
 {
@@ -142,16 +144,17 @@ TextureChild::Recv__delete__()
 
 void
 TextureChild::ActorDestroy(ActorDestroyReason why)
 {
   if (mTextureClient) {
     mTextureClient->mActor = nullptr;
   }
   mWaitForRecycle = nullptr;
+  delete mKeep;
 }
 
 // static
 PTextureChild*
 TextureClient::CreateIPDLActor()
 {
   TextureChild* c = new TextureChild();
   c->AddIPDLReference();
@@ -414,16 +417,24 @@ TextureClient::TextureClient(TextureFlag
 {}
 
 TextureClient::~TextureClient()
 {
   // All the destruction code that may lead to virtual method calls must
   // be in Finalize() which is called just before the destructor.
 }
 
+void
+TextureClient::KeepUntilFullDeallocation(KeepAlive* aKeep)
+{
+  MOZ_ASSERT(mActor);
+  MOZ_ASSERT(!mActor->mKeep);
+  mActor->mKeep = aKeep;
+}
+
 void TextureClient::ForceRemove()
 {
   if (mValid && mActor) {
     if (GetFlags() & TextureFlags::DEALLOCATE_CLIENT) {
       if (mActor->IPCOpen()) {
         mActor->SendClearTextureHostSync();
         mActor->SendRemoveTexture();
       }
--- a/gfx/layers/client/TextureClient.h
+++ b/gfx/layers/client/TextureClient.h
@@ -45,16 +45,17 @@ class ISurfaceAllocator;
 class CompositableClient;
 class PlanarYCbCrImage;
 struct PlanarYCbCrData;
 class Image;
 class PTextureChild;
 class TextureChild;
 class BufferTextureClient;
 class TextureClient;
+class KeepAlive;
 
 /**
  * TextureClient is the abstraction that allows us to share data between the
  * content and the compositor side.
  */
 
 enum TextureAllocationFlags {
   ALLOC_DEFAULT = 0,
@@ -293,16 +294,24 @@ public:
 
   /**
    * If this method returns false users of TextureClient are not allowed
    * to access the shared data.
    */
   bool IsValid() const { return mValid; }
 
   /**
+   * kee the passed object alive until the IPDL actor is destroyed. This can
+   * 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.
+   */
+  void KeepUntilFullDeallocation(KeepAlive* aKeep);
+
+  /**
    * Create and init the TextureChild/Parent IPDL actor pair.
    *
    * Should be called only once per TextureClient.
    */
   bool InitIPDLActor(CompositableForwarder* aForwarder);
 
   /**
    * Return a pointer to the IPDLActor.
@@ -608,11 +617,26 @@ struct TextureClientAutoUnlock
   : mTexture(aTexture) {}
 
   ~TextureClientAutoUnlock()
   {
     mTexture->Unlock();
   }
 };
 
+class KeepAlive
+{
+public:
+  virtual ~KeepAlive() {}
+};
+
+template<typename T>
+class TKeepAlive : public KeepAlive
+{
+public:
+  TKeepAlive(T* aData) : mData(aData) {}
+protected:
+  RefPtr<T> mData;
+};
+
 }
 }
 #endif
--- a/gfx/layers/d3d11/TextureD3D11.cpp
+++ b/gfx/layers/d3d11/TextureD3D11.cpp
@@ -162,16 +162,19 @@ TextureClientD3D11::TextureClientD3D11(g
   , mFormat(aFormat)
   , mIsLocked(false)
   , mNeedsClear(false)
   , mNeedsClearWhite(false)
 {}
 
 TextureClientD3D11::~TextureClientD3D11()
 {
+  if (mTexture && mActor) {
+    KeepUntilFullDeallocation(new TKeepAlive<ID3D10Texture2D>(mTexture));
+  }
 #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
   // shouldn't -really- need the lock but the debug layer chokes on this.
   if (mDrawTarget) {
     MOZ_ASSERT(!mIsLocked);
     MOZ_ASSERT(mTexture);
     MOZ_ASSERT(mDrawTarget->refCount() == 1);
@@ -305,51 +308,66 @@ TextureClientD3D11::ToSurfaceDescriptor(
 }
 
 DXGITextureHostD3D11::DXGITextureHostD3D11(TextureFlags aFlags,
                                            const SurfaceDescriptorD3D10& aDescriptor)
   : TextureHost(aFlags)
   , mHandle(aDescriptor.handle())
   , mFormat(aDescriptor.format())
   , mIsLocked(false)
-{}
+{
+  OpenSharedHandle();
+}
+
+bool
+DXGITextureHostD3D11::OpenSharedHandle()
+{
+  if (!GetDevice()) {
+    return false;
+  }
+
+  HRESULT hr = GetDevice()->OpenSharedResource((HANDLE)mHandle,
+                                               __uuidof(ID3D11Texture2D),
+                                               (void**)(ID3D11Texture2D**)byRef(mTexture));
+  if (FAILED(hr)) {
+    NS_WARNING("Failed to open shared texture");
+    return false;
+  }
+
+  D3D11_TEXTURE2D_DESC desc;
+  mTexture->GetDesc(&desc);
+  mSize = IntSize(desc.Width, desc.Height);
+  return true;
+}
 
 ID3D11Device*
 DXGITextureHostD3D11::GetDevice()
 {
-  return mCompositor ? mCompositor->GetDevice() : nullptr;
+  return gfxWindowsPlatform::GetPlatform()->GetD3D11Device();
 }
 
 void
 DXGITextureHostD3D11::SetCompositor(Compositor* aCompositor)
 {
   mCompositor = static_cast<CompositorD3D11*>(aCompositor);
 }
 
 bool
 DXGITextureHostD3D11::Lock()
 {
   if (!GetDevice()) {
     NS_WARNING("trying to lock a TextureHost without a D3D device");
     return false;
   }
   if (!mTextureSource) {
-    RefPtr<ID3D11Texture2D> tex;
-    HRESULT hr = GetDevice()->OpenSharedResource((HANDLE)mHandle,
-                                                 __uuidof(ID3D11Texture2D),
-                                                 (void**)(ID3D11Texture2D**)byRef(tex));
-    if (FAILED(hr)) {
-      NS_WARNING("Failed to open shared texture");
+    if (!mTexture && !OpenSharedHandle()) {
       return false;
     }
 
-    mTextureSource = new DataTextureSourceD3D11(mFormat, mCompositor, tex);
-    D3D11_TEXTURE2D_DESC desc;
-    tex->GetDesc(&desc);
-    mSize = IntSize(desc.Width, desc.Height);
+    mTextureSource = new DataTextureSourceD3D11(mFormat, mCompositor, mTexture);
   }
 
   mIsLocked = LockD3DTexture(mTextureSource->GetD3D11Texture());
 
   return mIsLocked;
 }
 
 void
--- a/gfx/layers/d3d11/TextureD3D11.h
+++ b/gfx/layers/d3d11/TextureD3D11.h
@@ -188,16 +188,19 @@ public:
   virtual TemporaryRef<gfx::DataSourceSurface> GetAsSurface() MOZ_OVERRIDE
   {
     return nullptr;
   }
 
 protected:
   ID3D11Device* GetDevice();
 
+  bool OpenSharedHandle();
+
+  RefPtr<ID3D11Texture2D> mTexture;
   RefPtr<DataTextureSourceD3D11> mTextureSource;
   RefPtr<CompositorD3D11> mCompositor;
   gfx::IntSize mSize;
   WindowsHandle mHandle;
   gfx::SurfaceFormat mFormat;
   bool mIsLocked;
 };