Implemented mitigation of a D3D12 debug layer bug in SetName method

4 files changed, 146 insertions, 5 deletions

diff --git a/UnitTests/DiligentCoreAPITest/CMakeLists.txt b/UnitTests/DiligentCoreAPITest/CMakeLists.txt
index 302946ec..7e96699f 100644
--- a/UnitTests/DiligentCoreAPITest/CMakeLists.txt
+++ b/UnitTests/DiligentCoreAPITest/CMakeLists.txt
@@ -38,6 +38,14 @@ if(PLATFORM_WIN32)
     list(APPEND SOURCE ${SOURCE_WIN32})
 endif()
 
+if(D3D12_SUPPORTED)
+    file(GLOB D3D12_SOURCE LIST_DIRECTORIES false src/D3D12/*)
+    file(GLOB D3D12_INCLUDE LIST_DIRECTORIES false include/D3D12/*)
+    list(APPEND INCLUDE ${D3D12_INCLUDE})
+    list(APPEND SOURCE ${D3D12_SOURCE})
+endif()
+
+
 add_executable(DiligentCoreAPITest ${SOURCE} ${INCLUDE} ${SHADERS})
 set_common_target_properties(DiligentCoreAPITest)
 
diff --git a/UnitTests/DiligentCoreAPITest/include/D3D12/D3D12DebugLayerSetNameBugWorkaround.h b/UnitTests/DiligentCoreAPITest/include/D3D12/D3D12DebugLayerSetNameBugWorkaround.h
new file mode 100644
index 00000000..9243cdf7
--- /dev/null
+++ b/UnitTests/DiligentCoreAPITest/include/D3D12/D3D12DebugLayerSetNameBugWorkaround.h
@@ -0,0 +1,52 @@
+/*     Copyright 2019 Diligent Graphics LLC
+ *  
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF ANY PROPRIETARY RIGHTS.
+ *
+ *  In no event and under no legal theory, whether in tort (including negligence), 
+ *  contract, or otherwise, unless required by applicable law (such as deliberate 
+ *  and grossly negligent acts) or agreed to in writing, shall any Contributor be
+ *  liable for any damages, including any direct, indirect, special, incidental, 
+ *  or consequential damages of any character arising as a result of this License or 
+ *  out of the use or inability to use the software (including but not limited to damages 
+ *  for loss of goodwill, work stoppage, computer failure or malfunction, or any and 
+ *  all other commercial damages or losses), even if such Contributor has been advised 
+ *  of the possibility of such damages.
+ */
+
+#include <memory>
+
+#include "RenderDevice.h"
+
+namespace Diligent
+{
+
+// There is a bug in D3D12 debug layer as of build version 10.0.18362: SetName() method
+// is not protected by a mutex internally. This, in combination with the fact that root signatures are
+// de-duplicated by D3D12 runtime, causes the following problem:
+// when multiple threads attempt to create the same root signature, the run-time returns the same
+// object. Calling SetName() results in a race condition because multiple threads attempt to resize/write
+// to the buffer without a mutex.
+
+// As a workaround, we create the root signature ahead of time and reserve enough space for the name
+// to avoid memory allocation.
+
+class D3D12DebugLayerSetNameBugWorkaround
+{
+public:
+    explicit D3D12DebugLayerSetNameBugWorkaround(IRenderDevice* pDevice);
+    ~D3D12DebugLayerSetNameBugWorkaround();
+
+private:
+    struct RootSignatureWrapper;
+    std::unique_ptr<RootSignatureWrapper> m_RootSignature;
+};
+
+} // namespace Diligent
diff --git a/UnitTests/DiligentCoreAPITest/src/D3D12/D3D12DebugLayerSetNameBugWorkaround.cpp b/UnitTests/DiligentCoreAPITest/src/D3D12/D3D12DebugLayerSetNameBugWorkaround.cpp
new file mode 100644
index 00000000..b0ef512f
--- /dev/null
+++ b/UnitTests/DiligentCoreAPITest/src/D3D12/D3D12DebugLayerSetNameBugWorkaround.cpp
@@ -0,0 +1,71 @@
+/*     Copyright 2019 Diligent Graphics LLC
+ *  
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF ANY PROPRIETARY RIGHTS.
+ *
+ *  In no event and under no legal theory, whether in tort (including negligence), 
+ *  contract, or otherwise, unless required by applicable law (such as deliberate 
+ *  and grossly negligent acts) or agreed to in writing, shall any Contributor be
+ *  liable for any damages, including any direct, indirect, special, incidental, 
+ *  or consequential damages of any character arising as a result of this License or 
+ *  out of the use or inability to use the software (including but not limited to damages 
+ *  for loss of goodwill, work stoppage, computer failure or malfunction, or any and 
+ *  all other commercial damages or losses), even if such Contributor has been advised 
+ *  of the possibility of such damages.
+ */
+
+#include "../include/D3D12/D3D12DebugLayerSetNameBugWorkaround.h"
+
+#include "RefCntAutoPtr.h"
+
+#ifndef NOMINMAX
+#    define NOMINMAX
+#endif
+
+#include <d3d12.h>
+#include <atlcomcli.h>
+
+#include "RenderDeviceD3D12.h"
+
+namespace Diligent
+{
+
+struct D3D12DebugLayerSetNameBugWorkaround::RootSignatureWrapper
+{
+    CComPtr<ID3D12RootSignature> pRootSignature;
+};
+
+D3D12DebugLayerSetNameBugWorkaround::D3D12DebugLayerSetNameBugWorkaround(IRenderDevice* pDevice) :
+    m_RootSignature(new RootSignatureWrapper)
+{
+    if (pDevice->GetDeviceCaps().DevType != DeviceType::D3D12)
+        return;
+
+    RefCntAutoPtr<IRenderDeviceD3D12> pDeviceD3D12(pDevice, IID_RenderDeviceD3D12);
+    VERIFY_EXPR(pDeviceD3D12);
+    auto pd3d12Device = pDeviceD3D12->GetD3D12Device();
+
+    D3D12_ROOT_SIGNATURE_DESC RootSignatureDesc = {};
+
+    RootSignatureDesc.Flags = D3D12_ROOT_SIGNATURE_FLAG_ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT;
+
+    CComPtr<ID3DBlob> signature;
+    D3D12SerializeRootSignature(&RootSignatureDesc, D3D_ROOT_SIGNATURE_VERSION_1, &signature, nullptr);
+    auto& pRootSign = m_RootSignature->pRootSignature;
+    pd3d12Device->CreateRootSignature(0, signature->GetBufferPointer(), signature->GetBufferSize(), __uuidof(pRootSign), reinterpret_cast<void**>(static_cast<ID3D12RootSignature**>(&pRootSign)));
+    pRootSign->SetName(L"A long string to make sure there is enough space reserved in the buffer to avoid resize when SetName is called "
+                       "and it is accessed simultaneously from multiple threads without a mutex");
+}
+
+D3D12DebugLayerSetNameBugWorkaround::~D3D12DebugLayerSetNameBugWorkaround()
+{
+}
+
+} // namespace Diligent
diff --git a/UnitTests/DiligentCoreAPITest/src/MTResourceCreationTest.cpp b/UnitTests/DiligentCoreAPITest/src/MTResourceCreationTest.cpp
index 75934401..7e105fdc 100644
--- a/UnitTests/DiligentCoreAPITest/src/MTResourceCreationTest.cpp
+++ b/UnitTests/DiligentCoreAPITest/src/MTResourceCreationTest.cpp
@@ -27,6 +27,9 @@
 
 #include "TestingEnvironment.h"
 #include "ThreadSignal.h"
+#if D3D12_SUPPORTED
+#    include "D3D12/D3D12DebugLayerSetNameBugWorkaround.h"
+#endif
 
 #include "gtest/gtest.h"
 
@@ -69,7 +72,7 @@ protected:
 
     static const int NumBuffersToCreate  = 10;
     static const int NumTexturesToCreate = 5;
-    static const int NumPSOToCreate      = 2;
+    static const int NumPSOToCreate      = 3;
 
 #ifdef _DEBUG
     static const int NumIterations = 10;
@@ -266,10 +269,6 @@ void MultithreadedResourceCreationTest::WorkerThreadFunc(MultithreadedResourceCr
 
 TEST_F(MultithreadedResourceCreationTest, CreateResources)
 {
-    // This test sometimes randomly crashes when running in Direct3D12 mode (both SW and HW) which is caused by
-    // a bug in D3D12 debug layer. Apparently SetName() method works incorrectly in a multithreading environment.
-    // Disabling the debug layer or limiting the length of the name by 15 symbols fixes the problem.
-
     auto* pEnv    = TestingEnvironment::GetInstance();
     auto* pDevice = pEnv->GetDevice();
     if (pDevice->GetDeviceCaps().IsGLDevice())
@@ -277,6 +276,17 @@ TEST_F(MultithreadedResourceCreationTest, CreateResources)
         GTEST_SKIP() << "Multithreading resource creation is not supported in OpenGL";
     }
 
+#if D3D12_SUPPORTED
+    // There is a bug in D3D12 debug layer as of build version 10.0.18362: SetName() method
+    // is not protected by a mutex internally. This, in combination with the fact that root signatures are
+    // de-duplicated by D3D12 runtime results in a race condition when SetName is called and causes random crashes.
+
+    // As a workaround, we create the root signature ahead of time and reserve enough space for the name
+    // to avoid memory allocation.
+
+    D3D12DebugLayerSetNameBugWorkaround D3D12DebugLayerBugWorkaround(pDevice);
+#endif
+
     TestingEnvironment::ScopedReleaseResources AutoResetEnvironment;
 
     auto numCores = std::thread::hardware_concurrency();