How to correctly release `ID3D12Resource` for `IDXGISwapChain3::ResizeBuffers`

[polymonster]

Hi I'm new to rust, but experienced with win32/directx.

I am trying to make a resizable swap chain from a window using IDXGISwapChain3::ResizeBuffers

I get an error in the validation layer DXGI ERROR: IDXGISwapChain::ResizeBuffers: Swapchain cannot be resized unless all outstanding buffer references have been released. [ MISCELLANEOUS ERROR #19: ]

I have working c++ code which calls ID3D12Resource::Release on buffers which are obtained from the IDXGISwapChain3::GetBuffer function before making the call to IDXGISwapChain3::ResizeBuffers.

What would be the equivalent of a Release call in rust? because the api doesn't mention Release

[robmikh]

AddRef and Release are tied to Clone and Drop respectively. Implicitly or explicitly dropping your buffers should be sufficient.

[polymonster]

Thanks for your quick response,

In this case I don't have references to buffers that I keep myself, they are created implicitly along with the IDXGISwapChain3 so I think I must need to explicitly drop them as the ownership is with the swap chain and I want to retain the swap chain itself.

I am not sure how to explicitly drop the internally owned buffer resources. They can be obtained through the function call IDXGISwapChain3::GetBuffer I have tried passing to std::mem::drop but still get the same validation error.

  for i in 0..FRAME_COUNT {
      let buffer: ID3D12Resource = self.swap_chain.GetBuffer(i).unwrap();
      std::mem::drop(buffer);
  }

Thanks again for your help, it has given me more to look into.

[robmikh]

Have you created render target views for any of the buffers? I believe they'll hold a reference.

[polymonster]

Yeah, I have references to render target views which I was also clearing.

I have been able to successfully call ResizeBuffers as long as I don't use the render target views in a command list, so I'm just following back from there now to make sure the waits are being correctly handled and the command lists are no longer in use on the gpu.

I think it should be ok from here on out, at first I wasn't entirely sure how the COM objects interacted with rust lifetimes but it's much clearer now.

Thanks for the help.

[polymonster]

The issue seems to be with using a transition barrier, each frame the swap chain buffer leaks references. This issue is also present in the direct3d12 sample of the repository.

If you build run the app through visual studio upon closing D3D Live objects are reported in the debug console.

D3D12 WARNING: 	Live Object at 0x000001F4E2D4D020, Refcount: 1318. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x000001F4E2D4CD70, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x000001F4E2D4DFD0, Refcount: 1318. [ STATE_CREATION WARNING #0: UNKNOWN]

The 2 with large ref counts in the samples case leak 2 references per frame. one barrier for transitioning present > write and then write > present.

I am able to successfully resize the swap chain (and make draw calls, clear) without using the transition (with validation warnings spewing out)

The command lists and allocators have been reset and I have also tried to create new command lists but that also doesn't seem to help to drop the refs.

I am not sure at this point whose responsibility it is to drop the memory for the transition. I am using the same function the sample in this repo uses.

fn transition_barrier(
    resource: &ID3D12Resource,
    state_before: D3D12_RESOURCE_STATES,
    state_after: D3D12_RESOURCE_STATES,
) -> D3D12_RESOURCE_BARRIER {
    D3D12_RESOURCE_BARRIER {
        Type: D3D12_RESOURCE_BARRIER_TYPE_TRANSITION,
        Flags: D3D12_RESOURCE_BARRIER_FLAG_NONE,
        Anonymous: D3D12_RESOURCE_BARRIER_0 {
            Transition: std::mem::ManuallyDrop::new(D3D12_RESOURCE_TRANSITION_BARRIER {
                pResource: Some(resource.clone()),
                StateBefore: state_before,
                StateAfter: state_after,
                Subresource: D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES,
            }),
        },
    }
}

The transition in the barrier is std::mem::ManuallyDrop so this makes sense the references might leak, I just haven't figured out how to call manually drop myself because I am new to rust and there are quite a lot of new concepts this is covering.

I will continue digging and see where I get.

[polymonster]

OK I have managed to get it working, but it doesn't quite feel right. I can manually drop the transition immediately after pushing the resource barrier into a command list:

let mut barrier = transition_barrier(
    &queue.swap_chain.GetBuffer(bb as u32).unwrap(),
    D3D12_RESOURCE_STATE_RENDER_TARGET,
    D3D12_RESOURCE_STATE_PRESENT,
);
self.command_list[bb].ResourceBarrier(1, &barrier);
let _: D3D12_RESOURCE_TRANSITION_BARRIER = std::mem::ManuallyDrop::into_inner(barrier.Anonymous.Transition);

This cleans up all references and when closing the app no D3D Live Objects are being reported.

The extra book-keeping is not necessary with barriers in c++. I am now just not 100% sure how to handle this correctly, it works right now with no issues or any validation errors but I'm not sure when the drop should actually be called. After the transition is passed to command list? after the command list is submitted on CPU? or after command list is submitted on the GPU?

As the sample for direct3d12 has this leak issue it would be useful to have a correct implementation of where to drop the transition.

[polymonster]

I am going to close this issue no I have investigated a bit more.

For each barrier transition added to a command list I push a barrier (move) into a vec. Once the command list has been completed on the GPU the barriers are removed from the vec and then manually dropped.

This will ensure all in flight resources on the GPU will retain a reference and allows to decouple CPU resource management from the GPU.

[SK83RJOSH]

Ran into this same issue and don't think this should be closed but perhaps be something reevaluated by @kennykerr on the code gen side? The ManuallyDrop here is pretty dangerous and it took me a full day to figure out what was going on in my application. I know this API is unsafe in general, but this is a pretty sharp edge to cut yourself on.

[polymonster]

Yeah I agree, I spent a good couple of days debugging this.

I am new to rust and it was the first time I encountered ManuallyDrop so wasn't sure if it was just me who was confused and not sure how to deal with it.

I think the intention is that once you create a transition you must manually drop after it has been consumed on the GPU, so keeping a reference to the resource until you know it is safe to drop it makes sense.

I am not sure if this can be deduced or automatically handled because of how internally d3d12 works, but I think the ideal situation for a user would be that when a D3D12_RESOURCE_BARRIER is passed to a ID3D12GraphicsCommandList the command list takes ownership of the resource ref and then once the command buffer is Reset then it drops the resources that were attached to transitions.

I am doing this already myself, you can see in the implementation here... my CmdBuf keeps track of in_flight_barriers and drops them after swapping buffers and knowing the command list has been consumed.

[kennykerr]

Thanks for the reminder. I was planning on looking into this issue more closely and if the issue remains open, I won't forget about it. 😉

[kennykerr]

@damyanp / @robmikh any insights you can share about the ownership semantics expected by D3D would be much appreciated.

[SK83RJOSH]

I think the intention is that once you create a transition you must manually drop after it has been consumed on the GPU, so keeping a reference to the resource until you know it is safe to drop it makes sense.

This was something I checked the docs about but they didn't shed much light on the matter. One would imagine that D3D would increment the COM pointer internally and hold onto it, but perhaps wistful thinking. I any case I made the assumption the related resource would be kept alive by other means and drop it immediately. Works for now, but something I'll need to revisit for sure.

Cool stuff btw! Definitely will give me some inspiration for my project (porting egui to windows-rs). Thanks for sharing. 😄

Thanks for the reminder. I was planning on looking into this issue more closely and if the issue remains open, I won't forget about it. 😉

No problem! I figured it was something that'd be of interest and just slipped through the cracks. I noticed that it seems all of Damyan's sample ports are hit by this issue too, so I feel somewhat validated in my struggle to track this down now. Hopefully you find a nice solution in the end. 🤞

Thanks for all the great work on windows-rs and C++/WinRT by the way. You're making Windows programming a lot more fun. 😁

[damyanp]

D3D12 is fundamentally unsafe here and relies on the programmer to ensure that resources are kept alive while the GPU is still using them. It does not increase the reference counts for resources passed to it through resource barriers, or for any other APIs that record command lists or create descriptors.

So, in terms of COM reference counts, D3D does not take ownership of the resources passed in to these APIs. Ideally, the projection would allow the app to just pass in an unsafe raw pointer to D3D. IIRC, that was possible in an earlier version of windows-rs with the ABI types?

[kennykerr]

Thanks Damyan,

Ideally, the projection would allow the app to just pass in an unsafe raw pointer to D3D.

That is consistent with how the windows crate uses ManuallyDrop to put the ownership and decision-making here in the caller's hands.

[damyanp]

Ideally, though, the transition barrier struct itself shouldn't own the reference. It should be possible to fill out the struct without an addref.

[kennykerr]

Yes, unfortunately there's no metadata to distinguish between structs that want to own their fields and structs that don't. If we can convince the Win32 metadata folks to add something like that, I could plumb it through to Rust.

[kennykerr]

There isn't really anything left for me to do here. Perhaps @sotteson1 or @riverar want to tackle this in the metadata as I suggested above.

[kennykerr]

Reopening this as I had the idea to introduce a Borrowed<T> type that can be used to solve this and provide lifetime semantics more in line with Win32.

[sotteson1]

There isn't really anything left for me to do here. Perhaps @sotteson1 or @riverar want to tackle this in the metadata as I suggested above.

I don't know of a way for the scraper to know which structs own their fields and which don't. I can see the benefit, but I don't know where I would get that kind of information.

[kennykerr]

WinRT structs definitely own their fields while Win32 structs traditionally don't and just hold on to raw pointers/handles without implying that those resources will be freed when the struct goes out of scope. I've got a new Borrowed<T> type that I think should fix this for Win32 structs (WinRT structs already do the right thing) and also solve a few other scenarios that require borrowing owned resources. Borrowed is basically a lifetime-aware borrow-friendly ABI-compatible version of ManuallyDrop. It also comes in handy for interop when directly working with the ABI for some API.

[riverar]

@sotteson1 Think we'll have to manually curate a list to track these.

@kennykerr Is this what you're effectively looking for in metadata?

// https://docs.microsoft.com/windows/win32/api/d3d12/ns-d3d12-d3d12_resource_transition_barrier
public struct D3D12_RESOURCE_TRANSITION_BARRIER
{
+ [Borrowed]
  public ID3D12Resource pResource;
  public uint Subresource;
  public D3D12_RESOURCE_STATES StateBefore;
  public D3D12_RESOURCE_STATES StateAfter;
}

// https://docs.microsoft.com/windows/win32/wmdm/wmdm-prop-config
public struct WMDM_PROP_CONFIG
{
  public uint nPreference;
  public uint nPropDesc;
+ [Owned]
  public unsafe WMDM_PROP_DESC* pPropDesc;
}

[kennykerr]

I don't think we should try to encode this in metadata. We should instead just assume that all Win32 structs don't own. That's what the Borrowed<T> type provides - it tracks lifetime of borrowed values so there's no dangling references without actually forcing each struct to drop its fields.

[kennykerr]

This would also solve #907.

[rylev]

It would be great if anyone who is interested in this issue to take a look at https://github.com/microsoft/windows-rs/pull/1788 which I believe would address this.