From b44ddde559e876ee0d7ecc0f4df76e70c30655bc Mon Sep 17 00:00:00 2001 From: assiduous Date: Fri, 13 Dec 2019 20:01:10 -0800 Subject: Implemented mitigation of a D3D12 debug layer bug in SetName method --- UnitTests/DiligentCoreAPITest/CMakeLists.txt | 8 +++ .../D3D12/D3D12DebugLayerSetNameBugWorkaround.h | 52 ++++++++++++++++ .../D3D12/D3D12DebugLayerSetNameBugWorkaround.cpp | 71 ++++++++++++++++++++++ .../src/MTResourceCreationTest.cpp | 20 ++++-- 4 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 UnitTests/DiligentCoreAPITest/include/D3D12/D3D12DebugLayerSetNameBugWorkaround.h create mode 100644 UnitTests/DiligentCoreAPITest/src/D3D12/D3D12DebugLayerSetNameBugWorkaround.cpp (limited to 'UnitTests') 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 + +#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 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 +#include + +#include "RenderDeviceD3D12.h" + +namespace Diligent +{ + +struct D3D12DebugLayerSetNameBugWorkaround::RootSignatureWrapper +{ + CComPtr pRootSignature; +}; + +D3D12DebugLayerSetNameBugWorkaround::D3D12DebugLayerSetNameBugWorkaround(IRenderDevice* pDevice) : + m_RootSignature(new RootSignatureWrapper) +{ + if (pDevice->GetDeviceCaps().DevType != DeviceType::D3D12) + return; + + RefCntAutoPtr 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 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(static_cast(&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(); -- cgit v1.2.3