Leak! Pool of framebuffers in `wgpu_hal::vulkan::CommandEncoder` grows unbounded

[vntec]

Initially I noticed that the RAM usage of my program was growing ever so slightly (~0.5 MB/s).
But after hours of debugging my renderer, I think I may have found a leak in wgpu-hal.
I could reproduce the behavior in the official wgpu examples, like the "shadow" example.

Description
It seems like the framebuffers hash map in the wgpu_hal::vulkan::CommandEncoder is never reduced in size. This is problematic since hal command encoders are pooled and reused. The surface is updated each frame, so the FramebufferKey never matches any of the old framebuffers. A new one is created each frame and the memory consumption grows unbounded.

Repro steps
Clone the wgpu trunk, run the shadow example on Vulkan and set the present mode to Immediate (inside resume in framework.rs). Observe the used memory or add dbg!(self.framebuffers.len()); to the begin_render_pass function in wgpu-hal/src/vulkan/command.rs and see how the hash map length increases.

Expected vs observed behavior
I expected the pooled framebuffers to be destroyed after some time, or maybe when a length limitation is reached.

Extra materials
This console output should be sufficient [wgpu-hal/src/vulkan/command.rs:772:2] self.framebuffers.len() = 109383 :D

Platform
wgpu trunk, Ubuntu 22.04.02, Nvidia 570.153.02

Please excuse me if I missed something, this is my first issue and my knowledge of the wgpu code base is limited.

[sagudev]

Indeed. There is no code to remove hashmap entries except drain from CommandEncoder::drop.

The comment says that

wgpu/wgpu-hal/src/vulkan/mod.rs

Lines 15 to 16 in 1280819

	Framebuffers are also cached on the device, but they are removed when
	any of the image views (they have) gets removed.

historically there was this:

wgpu/wgpu-hal/src/vulkan/device.rs

Line 753 in 45074b2

let mut fbuf_lock = self.shared.framebuffers.lock();

[vntec]

Ok, that's interesting insight! But keeping the entries forever can't be intentional, right?

[vntec]

In addition, temp_texture_views are also only drained in CommandEncoder::drop.
As this drop fn is the only occurrence where I could find a call to ash::Device::destroy_framebuffer, this seems like a major oversight that needs to be fixed!

Is there any mechanism already implemented that can determine if a Vulkan framebuffer is no longer required?
If not, a limited hash map size with LRU-like replacement could maybe work too.

I also couldn't find anything related to "If Vulkan supports image-less framebuffers,
then the actual views are excluded from the framebuffer key." in the implementation. begin_render_pass seems to always push attachments to the FramebufferKey.

Am I missing something, guys? Any help appreciated! :)

[jimblandy]

Yeah, it seems like we need to have a method that would somehow reset or trim command buffers before we save them for recycling.