Fixed stray dx12 hook in Tr2ResourceHelper from allowing devices to r… - #65
Fixed stray dx12 hook in Tr2ResourceHelper from allowing devices to r…#65JohnGreenFC wants to merge 1 commit into
Conversation
…ecover Tr2ResourceHelper leaked one upload-scratch reference via m_mapped (Destroy never cleared it; the synchronized UpdateBuffer path left it set), keeping the removed ID3D12Device alive so D3D12CreateDevice returned it forever and device-lost recovery looped indefinitely. Also: - Skip Aftermath DX12 init when the debug layer is on: it always fails there and SDK 2021.1's failure path leaks a device reference. - DRED enablement split out of EnableDebugLayer; on with the debug layer or the new /dred=1 argument. - Fixed DRED breadcrumb op listing never printing for hangs in the first 100 ops of a command list (unsigned underflow), and context strings being skipped.
|
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a DX12 device-lost recovery failure caused by a leaked upload-scratch reference in Tr2ResourceHelper, which could keep a removed ID3D12Device alive and cause D3D12CreateDevice to repeatedly return the same removed device.
Changes:
- Clear and release
m_mappedinTr2ResourceHelperon bothDestroy()and the synchronizedUpdateBuffer()path to prevent lingering COM references. - Add an opt-in
dredstartup arg (and global flag) and ensure DRED is enabled early in DX12 device creation when requested (or when using the debug layer). - Prevent Aftermath crash tracker initialization when the DX12 debug layer is enabled, and fix DRED breadcrumb logging bounds handling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| trinityal/dx12/Tr2ResourceHelper.cpp | Releases/clears m_mapped to eliminate a lingering upload resource reference that could keep a removed device alive. |
| trinityal/dx12/Tr2PrimaryRenderContextDx12.cpp | Enables DRED before device creation and avoids Aftermath init when debug layer is active to prevent device reference retention. |
| trinityal/ALResult.cpp | Adds the new global g_requestDred flag default. |
| trinity/trinity.cpp | Parses the dred startup argument to control DRED enablement. |
| trinity/TriDevice12.cpp | Fixes breadcrumb index math and context collection for DRED device-removed logging. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| m_gpuCrashTracker = nullptr; | ||
| } | ||
| else | ||
| else if( !hasDebugLayer ) |
There was a problem hiding this comment.
https://developer.nvidia.com/blog/advanced-api-performance-debugging/
Alternatively, use the DirectX 12 cross-vendor solution:
Use ID3D12GraphicsCommandList2::WriteBufferImmediate or DRED.
It isn’t supported in conjunction with Nsight Aftermath, so it is better to avoid mixing these.
What happens if hasDebugLayer is false and g_requestDred is true?
Tr2ResourceHelper leaked one upload-scratch reference via m_mapped (Destroy never cleared it; the synchronized UpdateBuffer path left it set), keeping the removed ID3D12Device alive so D3D12CreateDevice returned it forever and device-lost recovery looped indefinitely.