Bug 1974174 - Ensure we don't release the fences holder too early. r=sotaro,gfx-reviewers, a=dsmith

This patch adds an additional reference count to each FencesHolder
stored in the map, so that TextureHosts can acquire a reference at
initialization and release it at destruction. Previously the lifetime of
the entry was controlled by the TextureClient + TextureData.

Differential Revision: https://phabricator.services.mozilla.com/D255536

gfx/layers/d3d11/CompositeProcessD3D11FencesHolderMap.cpp

@@ -34,19 +34,51 @@ CompositeProcessD3D11FencesHolderMap::~CompositeProcessD3D11FencesHolderMap() {}
 
 void CompositeProcessD3D11FencesHolderMap::Register(
     CompositeProcessFencesHolderId aHolderId) {
+  MOZ_ASSERT(aHolderId.IsValid());
+
   MonitorAutoLock lock(mMonitor);
 
-  mFencesHolderById[aHolderId] = MakeUnique<FencesHolder>();
+  DebugOnly<bool> inserted =
+      mFencesHolderById.emplace(aHolderId, MakeUnique<FencesHolder>()).second;
+  MOZ_ASSERT(inserted, "Map already contained FencesHolder for id!");
 }
-void CompositeProcessD3D11FencesHolderMap::Unregister(
+
+void CompositeProcessD3D11FencesHolderMap::RegisterReference(
     CompositeProcessFencesHolderId aHolderId) {
+  if (!aHolderId.IsValid()) {
+    return;
+  }
+
   MonitorAutoLock lock(mMonitor);
 
   auto it = mFencesHolderById.find(aHolderId);
   if (it == mFencesHolderById.end()) {
+    MOZ_ASSERT_UNREACHABLE("Map missing FencesHolder for id!");
     return;
   }
-  mFencesHolderById.erase(it);
+
+  MOZ_ASSERT(it->second->mOwners > 0);
+  ++it->second->mOwners;
+}
+
+void CompositeProcessD3D11FencesHolderMap::Unregister(
+    CompositeProcessFencesHolderId aHolderId) {
+  if (!aHolderId.IsValid()) {
+    return;
+  }
+
+  MonitorAutoLock lock(mMonitor);
+
+  auto it = mFencesHolderById.find(aHolderId);
+  if (it == mFencesHolderById.end()) {
+    MOZ_ASSERT_UNREACHABLE("Map missing FencesHolder for id!");
+    return;
+  }
+
+  MOZ_ASSERT(it->second->mOwners > 0);
+  if (--it->second->mOwners == 0) {
+    mFencesHolderById.erase(it);
+  }
 }
 
 void CompositeProcessD3D11FencesHolderMap::SetWriteFence(
@@ -59,6 +91,7 @@ void CompositeProcessD3D11FencesHolderMap::SetWriteFence(
 
   MonitorAutoLock lock(mMonitor);
 
+  MOZ_ASSERT(aHolderId.IsValid());
   auto it = mFencesHolderById.find(aHolderId);
   if (it == mFencesHolderById.end()) {
     MOZ_ASSERT_UNREACHABLE("unexpected to be called");
@@ -87,6 +120,7 @@ void CompositeProcessD3D11FencesHolderMap::SetReadFence(
 
   MonitorAutoLock lock(mMonitor);
 
+  MOZ_ASSERT(aHolderId.IsValid());
   auto it = mFencesHolderById.find(aHolderId);
   if (it == mFencesHolderById.end()) {
     MOZ_ASSERT_UNREACHABLE("unexpected to be called");
@@ -114,6 +148,7 @@ bool CompositeProcessD3D11FencesHolderMap::WaitWriteFence(
   {
     MonitorAutoLock lock(mMonitor);
 
+    MOZ_ASSERT(aHolderId.IsValid());
     auto it = mFencesHolderById.find(aHolderId);
     if (it == mFencesHolderById.end()) {
       MOZ_ASSERT_UNREACHABLE("unexpected to be called");
@@ -142,6 +177,7 @@ bool CompositeProcessD3D11FencesHolderMap::WaitAllFencesAndForget(
   {
     MonitorAutoLock lock(mMonitor);
 
+    MOZ_ASSERT(aHolderId.IsValid());
     auto it = mFencesHolderById.find(aHolderId);
     if (it == mFencesHolderById.end()) {
       MOZ_ASSERT_UNREACHABLE("unexpected to be called");

gfx/layers/d3d11/CompositeProcessD3D11FencesHolderMap.h

@@ -33,6 +33,7 @@ class CompositeProcessD3D11FencesHolderMap {
   ~CompositeProcessD3D11FencesHolderMap();
 
   void Register(CompositeProcessFencesHolderId aHolderId);
+  void RegisterReference(CompositeProcessFencesHolderId aHolderId);
   void Unregister(CompositeProcessFencesHolderId aHolderId);
 
   void SetWriteFence(CompositeProcessFencesHolderId aHolderId,
@@ -51,13 +52,14 @@ class CompositeProcessD3D11FencesHolderMap {
 
     RefPtr<FenceD3D11> mWriteFence;
     std::vector<RefPtr<FenceD3D11>> mReadFences;
+    uint32_t mOwners = 1;
   };
 
-  mutable Monitor mMonitor MOZ_UNANNOTATED;
+  mutable Monitor mMonitor;
 
   std::unordered_map<CompositeProcessFencesHolderId, UniquePtr<FencesHolder>,
                      CompositeProcessFencesHolderId::HashFn>
-      mFencesHolderById;
+      mFencesHolderById MOZ_GUARDED_BY(mMonitor);
 
   static StaticAutoPtr<CompositeProcessD3D11FencesHolderMap> sInstance;
 };

gfx/layers/d3d11/TextureD3D11.cpp

@@ -359,6 +359,7 @@ D3D11TextureData::~D3D11TextureData() {
     }
   }
   if (mFencesHolderId.isSome()) {
+    MOZ_ASSERT(mFencesHolderId->IsValid());
     auto* fencesHolderMap = CompositeProcessD3D11FencesHolderMap::Get();
     if (fencesHolderMap) {
       fencesHolderMap->Unregister(mFencesHolderId.ref());
@@ -371,6 +372,7 @@ D3D11TextureData::~D3D11TextureData() {
 
 bool D3D11TextureData::Lock(OpenMode aMode) {
   if (mFencesHolderId.isSome()) {
+    MOZ_ASSERT(mFencesHolderId->IsValid());
     auto* fencesHolderMap = CompositeProcessD3D11FencesHolderMap::Get();
     fencesHolderMap->WaitAllFencesAndForget(mFencesHolderId.ref(), mDevice);
   }
@@ -414,6 +416,7 @@ bool D3D11TextureData::PrepareDrawTargetInLock(OpenMode aMode) {
 void D3D11TextureData::Unlock() {
   IncrementAndSignalWriteFence();
   if (mFencesHolderId.isSome()) {
+    MOZ_ASSERT(mFencesHolderId->IsValid());
     auto* fencesHolderMap = CompositeProcessD3D11FencesHolderMap::Get();
     fencesHolderMap->SetWriteFence(mFencesHolderId.ref(), mWriteFence);
   }
@@ -719,6 +722,9 @@ void D3D11TextureData::IncrementAndSignalWriteFence() {
   if (mFencesHolderId.isNothing() || !mWriteFence) {
     return;
   }
+
+  MOZ_ASSERT(mFencesHolderId->IsValid());
+
   auto* fencesHolderMap = CompositeProcessD3D11FencesHolderMap::Get();
   if (!fencesHolderMap) {
     MOZ_ASSERT_UNREACHABLE("unexpected to be called");
@@ -993,7 +999,28 @@ DXGITextureHostD3D11::DXGITextureHostD3D11(
       mHasKeyedMutex(aDescriptor.hasKeyedMutex()),
       mFencesHolderId(aDescriptor.fencesHolderId()),
       mColorSpace(aDescriptor.colorSpace()),
-      mColorRange(aDescriptor.colorRange()) {}
+      mColorRange(aDescriptor.colorRange()) {
+  if (!mFencesHolderId) {
+    return;
+  }
+  MOZ_ASSERT(mFencesHolderId->IsValid());
+  if (auto* fenceHolderMap = CompositeProcessD3D11FencesHolderMap::Get()) {
+    fenceHolderMap->RegisterReference(mFencesHolderId.ref());
+  } else {
+    MOZ_ASSERT_UNREACHABLE("FencesHolderMap not available");
+  }
+}
+
+DXGITextureHostD3D11::~DXGITextureHostD3D11() {
+  if (!mFencesHolderId) {
+    return;
+  }
+  if (auto* fenceHolderMap = CompositeProcessD3D11FencesHolderMap::Get()) {
+    fenceHolderMap->Unregister(mFencesHolderId.ref());
+  } else {
+    MOZ_ASSERT_UNREACHABLE("FencesHolderMap not available");
+  }
+}
 
 already_AddRefed<gfx::DataSourceSurface> DXGITextureHostD3D11::GetAsSurface(
     gfx::DataSourceSurface* aSurface) {
@@ -1445,11 +1472,24 @@ DXGIYCbCrTextureHostD3D11::DXGIYCbCrTextureHostD3D11(
       mYUVColorSpace(aDescriptor.yUVColorSpace()),
       mColorRange(aDescriptor.colorRange()),
       mFencesHolderId(aDescriptor.fencesHolderId()) {
+  if (auto* fenceHolderMap = CompositeProcessD3D11FencesHolderMap::Get()) {
+    fenceHolderMap->RegisterReference(mFencesHolderId);
+  } else {
+    MOZ_ASSERT_UNREACHABLE("FencesHolderMap not available");
+  }
   mHandles[0] = aDescriptor.handleY();
   mHandles[1] = aDescriptor.handleCb();
   mHandles[2] = aDescriptor.handleCr();
 }
 
+DXGIYCbCrTextureHostD3D11::~DXGIYCbCrTextureHostD3D11() {
+  if (auto* fenceHolderMap = CompositeProcessD3D11FencesHolderMap::Get()) {
+    fenceHolderMap->Unregister(mFencesHolderId);
+  } else {
+    MOZ_ASSERT_UNREACHABLE("FencesHolderMap not available");
+  }
+}
+
 void DXGIYCbCrTextureHostD3D11::CreateRenderTexture(
     const wr::ExternalImageId& aExternalImageId) {
   MOZ_ASSERT(mExternalImageId.isSome());

gfx/layers/d3d11/TextureD3D11.h

@@ -364,6 +364,7 @@ class DXGITextureHostD3D11 : public TextureHost {
  public:
   DXGITextureHostD3D11(TextureFlags aFlags,
                        const SurfaceDescriptorD3D10& aDescriptor);
+  ~DXGITextureHostD3D11() override;
 
   void DeallocateDeviceData() override {}
 
@@ -415,6 +416,7 @@ class DXGIYCbCrTextureHostD3D11 : public TextureHost {
  public:
   DXGIYCbCrTextureHostD3D11(TextureFlags aFlags,
                             const SurfaceDescriptorDXGIYCbCr& aDescriptor);
+  ~DXGIYCbCrTextureHostD3D11() override;
 
   void DeallocateDeviceData() override {}