From 217b584d15ad9d02389c689c77e2e2c6e7e16736 Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 4 Dec 2018 21:12:25 +0000 Subject: [PATCH] Fix D3D12 internal refcounting to ensure D3D12 device doesn't leak --- renderdoc/driver/d3d12/d3d12_command_list.h | 4 ++-- renderdoc/driver/d3d12/d3d12_commands.cpp | 5 +++++ renderdoc/driver/d3d12/d3d12_debug.cpp | 14 ++++++++++--- renderdoc/driver/d3d12/d3d12_device.cpp | 21 ++++++++++++++++++++ renderdoc/driver/d3d12/d3d12_device_wrap.cpp | 6 ++++++ renderdoc/driver/d3d12/d3d12_rendertext.cpp | 10 ++++++++++ 6 files changed, 55 insertions(+), 5 deletions(-) diff --git a/renderdoc/driver/d3d12/d3d12_command_list.h b/renderdoc/driver/d3d12/d3d12_command_list.h index 9edd2e3ee..2e2d6fd96 100644 --- a/renderdoc/driver/d3d12/d3d12_command_list.h +++ b/renderdoc/driver/d3d12/d3d12_command_list.h @@ -182,10 +182,10 @@ public: ////////////////////////////// // implement IUnknown - ULONG STDMETHODCALLTYPE AddRef() { return m_RefCounter.AddRef(); } + ULONG STDMETHODCALLTYPE AddRef() { return m_RefCounter.SoftRef(m_pDevice); } ULONG STDMETHODCALLTYPE Release() { - unsigned int ret = m_RefCounter.Release(); + unsigned int ret = m_RefCounter.SoftRelease(m_pDevice); if(ret == 0) delete this; return ret; diff --git a/renderdoc/driver/d3d12/d3d12_commands.cpp b/renderdoc/driver/d3d12/d3d12_commands.cpp index 6c1b28902..da98ea99c 100644 --- a/renderdoc/driver/d3d12/d3d12_commands.cpp +++ b/renderdoc/driver/d3d12/d3d12_commands.cpp @@ -238,6 +238,8 @@ WrappedID3D12CommandQueue::WrappedID3D12CommandQueue(ID3D12CommandQueue *real, WrappedID3D12CommandQueue::~WrappedID3D12CommandQueue() { + if(m_QueueRecord) + m_QueueRecord->Delete(m_pDevice->GetResourceManager()); m_pDevice->GetResourceManager()->ReleaseCurrentResource(GetResourceID()); m_pDevice->RemoveQueue(this); @@ -879,6 +881,9 @@ WrappedID3D12GraphicsCommandList2::~WrappedID3D12GraphicsCommandList2() if(m_ListRecord && m_ListRecord->bakedCommands) m_ListRecord->bakedCommands->Delete(m_pDevice->GetResourceManager()); + if(m_ListRecord) + m_ListRecord->Delete(m_pDevice->GetResourceManager()); + m_pDevice->GetResourceManager()->ReleaseCurrentResource(GetResourceID()); SAFE_RELEASE(m_WrappedDebug.m_pReal); diff --git a/renderdoc/driver/d3d12/d3d12_debug.cpp b/renderdoc/driver/d3d12/d3d12_debug.cpp index c63364aa7..dccb40d21 100644 --- a/renderdoc/driver/d3d12/d3d12_debug.cpp +++ b/renderdoc/driver/d3d12/d3d12_debug.cpp @@ -102,7 +102,6 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) RenderDoc::Inst().GetCrashHandler()->RegisterMemoryRegion(this, sizeof(D3D12DebugManager)); m_pDevice = wrapper; - m_pDevice->InternalRef(); D3D12ResourceManager *rm = wrapper->GetResourceManager(); @@ -117,6 +116,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) RDCCOMPILE_ASSERT(FIRST_WIN_RTV + 256 < 1024, "Increase size of RTV heap"); hr = m_pDevice->CreateDescriptorHeap(&desc, __uuidof(ID3D12DescriptorHeap), (void **)&rtvHeap); + m_pDevice->InternalRef(); if(FAILED(hr)) { @@ -131,6 +131,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) RDCCOMPILE_ASSERT(FIRST_WIN_DSV + 32 < 64, "Increase size of DSV heap"); hr = m_pDevice->CreateDescriptorHeap(&desc, __uuidof(ID3D12DescriptorHeap), (void **)&dsvHeap); + m_pDevice->InternalRef(); if(FAILED(hr)) { @@ -145,6 +146,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) RDCCOMPILE_ASSERT(MAX_SRV_SLOT < 4096, "Increase size of CBV/SRV/UAV heap"); hr = m_pDevice->CreateDescriptorHeap(&desc, __uuidof(ID3D12DescriptorHeap), (void **)&uavClearHeap); + m_pDevice->InternalRef(); if(FAILED(hr)) { @@ -157,6 +159,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) hr = m_pDevice->CreateDescriptorHeap(&desc, __uuidof(ID3D12DescriptorHeap), (void **)&cbvsrvuavHeap); + m_pDevice->InternalRef(); if(FAILED(hr)) { @@ -169,6 +172,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) desc.Type = D3D12_DESCRIPTOR_HEAP_TYPE_SAMPLER; hr = m_pDevice->CreateDescriptorHeap(&desc, __uuidof(ID3D12DescriptorHeap), (void **)&samplerHeap); + m_pDevice->InternalRef(); if(FAILED(hr)) { @@ -203,6 +207,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) m_RingConstantBuffer = MakeCBuffer(bufsize); m_RingConstantOffset = 0; + m_pDevice->InternalRef(); D3D12ShaderCache *shaderCache = m_pDevice->GetShaderCache(); @@ -223,6 +228,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) hr = m_pDevice->CreateRootSignature(0, root->GetBufferPointer(), root->GetBufferSize(), __uuidof(ID3D12RootSignature), (void **)&m_CBOnlyRootSig); + m_pDevice->InternalRef(); SAFE_RELEASE(root); @@ -243,6 +249,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) hr = m_pDevice->CreateRootSignature(0, root->GetBufferPointer(), root->GetBufferSize(), __uuidof(ID3D12RootSignature), (void **)&m_ArrayMSAARootSig); + m_pDevice->InternalRef(); SAFE_RELEASE(root); @@ -312,6 +319,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) hr = m_pDevice->CreateCommittedResource(&heapProps, D3D12_HEAP_FLAG_NONE, &readbackDesc, D3D12_RESOURCE_STATE_COPY_DEST, NULL, __uuidof(ID3D12Resource), (void **)&m_ReadbackBuffer); + m_pDevice->InternalRef(); m_ReadbackBuffer->SetName(L"m_ReadbackBuffer"); @@ -319,6 +327,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) hr = m_pDevice->CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, __uuidof(ID3D12CommandAllocator), (void **)&m_DebugAlloc); + m_pDevice->InternalRef(); if(FAILED(hr)) { @@ -332,6 +341,7 @@ D3D12DebugManager::D3D12DebugManager(WrappedID3D12Device *wrapper) hr = m_pDevice->CreateCommandList(0, D3D12_COMMAND_LIST_TYPE_DIRECT, m_DebugAlloc, NULL, __uuidof(ID3D12GraphicsCommandList), (void **)&list); + m_pDevice->InternalRef(); // safe to upcast - this is a wrapped object m_DebugList = (ID3D12GraphicsCommandList2 *)list; @@ -385,8 +395,6 @@ D3D12DebugManager::~D3D12DebugManager() SAFE_RELEASE(m_DebugAlloc); SAFE_RELEASE(m_DebugList); - m_pDevice->InternalRelease(); - if(RenderDoc::Inst().GetCrashHandler()) RenderDoc::Inst().GetCrashHandler()->UnregisterMemoryRegion(this); } diff --git a/renderdoc/driver/d3d12/d3d12_device.cpp b/renderdoc/driver/d3d12/d3d12_device.cpp index 9dee05623..23c22abc7 100644 --- a/renderdoc/driver/d3d12/d3d12_device.cpp +++ b/renderdoc/driver/d3d12/d3d12_device.cpp @@ -381,6 +381,15 @@ WrappedID3D12Device::~WrappedID3D12Device() { RenderDoc::Inst().RemoveDeviceFrameCapturer((ID3D12Device *)this); + if(!m_InternalCmds.pendingcmds.empty()) + ExecuteLists(m_Queue); + + if(!m_InternalCmds.submittedcmds.empty()) + FlushLists(true); + + for(size_t i = 0; i < m_InternalCmds.freecmds.size(); i++) + SAFE_RELEASE(m_InternalCmds.freecmds[i]); + if(!IsStructuredExporting(m_State)) SAFE_DELETE(WrappedID3D12Resource::m_List); @@ -400,6 +409,8 @@ WrappedID3D12Device::~WrappedID3D12Device() DestroyInternalResources(); + SAFE_RELEASE(m_Queue); + if(m_DeviceRecord) { RDCASSERT(m_DeviceRecord->GetRefCount() == 1); @@ -2253,7 +2264,9 @@ void WrappedID3D12Device::CreateInternalResources() CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, __uuidof(ID3D12CommandAllocator), (void **)&m_Alloc); + InternalRef(); CreateFence(0, D3D12_FENCE_FLAG_NONE, __uuidof(ID3D12Fence), (void **)&m_GPUSyncFence); + InternalRef(); m_GPUSyncHandle = ::CreateEvent(NULL, FALSE, FALSE, NULL); GetResourceManager()->SetInternalResource(m_Alloc); @@ -2261,11 +2274,13 @@ void WrappedID3D12Device::CreateInternalResources() CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, __uuidof(ID3D12CommandAllocator), (void **)&m_DataUploadAlloc); + InternalRef(); GetResourceManager()->SetInternalResource(m_DataUploadAlloc); CreateCommandList(0, D3D12_COMMAND_LIST_TYPE_DIRECT, m_DataUploadAlloc, NULL, __uuidof(ID3D12GraphicsCommandList), (void **)&m_DataUploadList); + InternalRef(); D3D12_DESCRIPTOR_HEAP_DESC desc; desc.Flags = D3D12_DESCRIPTOR_HEAP_FLAG_NONE; @@ -2274,6 +2289,7 @@ void WrappedID3D12Device::CreateInternalResources() desc.Type = D3D12_DESCRIPTOR_HEAP_TYPE_RTV; CreateDescriptorHeap(&desc, __uuidof(ID3D12DescriptorHeap), (void **)&m_RTVHeap); + InternalRef(); GetResourceManager()->SetInternalResource(m_RTVHeap); @@ -2376,6 +2392,7 @@ ID3D12GraphicsCommandList2 *WrappedID3D12Device::GetNewList() ID3D12GraphicsCommandList *list = NULL; HRESULT hr = CreateCommandList(0, D3D12_COMMAND_LIST_TYPE_DIRECT, m_Alloc, NULL, __uuidof(ID3D12GraphicsCommandList), (void **)&list); + InternalRef(); // safe to upcast because this is a wrapped object. ret = (ID3D12GraphicsCommandList2 *)list; @@ -2388,6 +2405,10 @@ ID3D12GraphicsCommandList2 *WrappedID3D12Device::GetNewList() if(IsReplayMode(m_State)) { GetResourceManager()->AddLiveResource(GetResID(ret), ret); + // add a reference here so that when we release our internal resources on destruction we don't + // free this too soon before the resource manager can. We still want to have it tracked as a + // resource in the manager though. + ret->AddRef(); } } diff --git a/renderdoc/driver/d3d12/d3d12_device_wrap.cpp b/renderdoc/driver/d3d12/d3d12_device_wrap.cpp index 6dcfa1dba..c20dd84e9 100644 --- a/renderdoc/driver/d3d12/d3d12_device_wrap.cpp +++ b/renderdoc/driver/d3d12/d3d12_device_wrap.cpp @@ -69,6 +69,9 @@ bool WrappedID3D12Device::Serialise_CreateCommandQueue(SerialiserType &ser, if(Descriptor.Type == D3D12_COMMAND_LIST_TYPE_DIRECT && m_Queue == NULL) { m_Queue = wrapped; + // we hold an extra ref on this during capture to keep it alive, for simplicity match that + // behaviour here. + m_Queue->AddRef(); CreateInternalResources(); } @@ -121,6 +124,9 @@ HRESULT WrappedID3D12Device::CreateCommandQueue(const D3D12_COMMAND_QUEUE_DESC * if(pDesc->Type == D3D12_COMMAND_LIST_TYPE_DIRECT && m_Queue == NULL) { m_Queue = wrapped; + // keep this queue alive even if the application frees it, for our own use + m_Queue->AddRef(); + InternalRef(); CreateInternalResources(); } diff --git a/renderdoc/driver/d3d12/d3d12_rendertext.cpp b/renderdoc/driver/d3d12/d3d12_rendertext.cpp index 9aa4ec111..50deddfe1 100644 --- a/renderdoc/driver/d3d12/d3d12_rendertext.cpp +++ b/renderdoc/driver/d3d12/d3d12_rendertext.cpp @@ -45,6 +45,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) desc.Type = D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV; hr = wrapper->CreateDescriptorHeap(&desc, __uuidof(ID3D12DescriptorHeap), (void **)&descHeap); + wrapper->InternalRef(); if(FAILED(hr)) { @@ -83,6 +84,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateCommittedResource(&uploadHeap, D3D12_HEAP_FLAG_NONE, &bufDesc, D3D12_RESOURCE_STATE_GENERIC_READ, NULL, __uuidof(ID3D12Resource), (void **)&uploadBuf); + // don't add InternalRef because this is temporary if(FAILED(hr)) RDCERR("Failed to create uploadBuf HRESULT: %s", ToStr(hr).c_str()); @@ -105,6 +107,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateCommittedResource(&defaultHeap, D3D12_HEAP_FLAG_NONE, &texDesc, D3D12_RESOURCE_STATE_COPY_DEST, NULL, __uuidof(ID3D12Resource), (void **)&Tex); + wrapper->InternalRef(); Tex->SetName(L"FontTex"); @@ -222,6 +225,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateCommittedResource(&heapProps, D3D12_HEAP_FLAG_NONE, &cbDesc, D3D12_RESOURCE_STATE_GENERIC_READ, NULL, __uuidof(ID3D12Resource), (void **)&GlyphData); + wrapper->InternalRef(); if(FAILED(hr)) RDCERR("Couldn't create GlyphData cbuffer! %s", ToStr(hr).c_str()); @@ -264,6 +268,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateCommittedResource(&heapProps, D3D12_HEAP_FLAG_NONE, &cbDesc, D3D12_RESOURCE_STATE_GENERIC_READ, NULL, __uuidof(ID3D12Resource), (void **)&Constants); + wrapper->InternalRef(); if(FAILED(hr)) RDCERR("Couldn't create Constants cbuffer! %s", ToStr(hr).c_str()); @@ -274,6 +279,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateCommittedResource(&heapProps, D3D12_HEAP_FLAG_NONE, &cbDesc, D3D12_RESOURCE_STATE_GENERIC_READ, NULL, __uuidof(ID3D12Resource), (void **)&CharBuffer); + wrapper->InternalRef(); if(FAILED(hr)) RDCERR("Couldn't create CharBuffer cbuffer! %s", ToStr(hr).c_str()); @@ -341,6 +347,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateRootSignature(0, root->GetBufferPointer(), root->GetBufferSize(), __uuidof(ID3D12RootSignature), (void **)&RootSig); + wrapper->InternalRef(); if(FAILED(hr)) RDCERR("Couldn't create RootSig! %s", ToStr(hr).c_str()); @@ -397,6 +404,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateGraphicsPipelineState(&pipeDesc, __uuidof(ID3D12PipelineState), (void **)&Pipe[BGRA8_BACKBUFFER]); + wrapper->InternalRef(); if(FAILED(hr)) RDCERR("Couldn't create BGRA8 Pipe! HRESULT: %s", ToStr(hr).c_str()); @@ -407,6 +415,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateGraphicsPipelineState(&pipeDesc, __uuidof(ID3D12PipelineState), (void **)&Pipe[RGBA8_BACKBUFFER]); + wrapper->InternalRef(); if(FAILED(hr)) RDCERR("Couldn't create RGBA8 Pipe! HRESULT: %s", ToStr(hr).c_str()); @@ -417,6 +426,7 @@ D3D12TextRenderer::D3D12TextRenderer(WrappedID3D12Device *wrapper) hr = wrapper->CreateGraphicsPipelineState(&pipeDesc, __uuidof(ID3D12PipelineState), (void **)&Pipe[RGBA16_BACKBUFFER]); + wrapper->InternalRef(); if(FAILED(hr)) RDCERR("Couldn't create RGBA16 Pipe! HRESULT: %s", ToStr(hr).c_str());