vulkano icon indicating copy to clipboard operation
vulkano copied to clipboard

Memory leak with CpuBufferPool on macOS

Open ggazebo opened this issue 7 years ago • 4 comments

Subbuffers allocated via a CpuBufferPool don't seem to be dropped properly on macOS. This eventually causes the app to crash with an overflow.

Issue can be seen with the teapot example, made more obvious using the change below.

diff --git a/examples/src/bin/teapot.rs b/examples/src/bin/teapot.rs
index ab3caa91..d10d4c3c 100644
--- a/examples/src/bin/teapot.rs
+++ b/examples/src/bin/teapot.rs
@@ -138,6 +138,8 @@ fn main() {
     let mut previous_frame = Box::new(vulkano::sync::now(device.clone())) as Box<GpuFuture>;
     let rotation_start = std::time::Instant::now();
 
+    let mut ub_cap = 0;
+
     loop {
         previous_frame.cleanup_finished();
 
@@ -192,6 +194,12 @@ fn main() {
             uniform_buffer.next(uniform_data).unwrap()
         };
 
+        let new_ub_cap = uniform_buffer.capacity();
+        if new_ub_cap > ub_cap {
+            ub_cap = new_ub_cap;
+            println!("ub capacity: {}", ub_cap);
+        }
+
         let set = Arc::new(vulkano::descriptor::descriptor_set::PersistentDescriptorSet::start(pipeline.clone(), 0)
             .add_buffer(uniform_buffer_subbuffer).unwrap()
             .build().unwrap()

I'm currently linking against MoltenVK 0.19.0.

Unsure if the issue exists when using the new 1.0 MoltenVK release. (I don't develop on macOS usually and at this point I don't know if I have 1.0 installed and linking correctly)

ggazebo avatar Mar 09 '18 03:03 ggazebo

Thanks for the issue. It is very unlikely that this an osx-only issue though. The code is totally cross platform.

tomaka avatar Mar 09 '18 09:03 tomaka

The same example does seem to work fine on Windows (different machine): capacity maxed out at 3 which seems correct.

Running with MoltenVK 1.0 results in the same behaviour.

AFAICT this is macOS only which is super bizzare.

ggazebo avatar Mar 09 '18 09:03 ggazebo

Actually waiting for the fence signal to return seems to workaround the issue. With the way vkWaitForFence() works, AFAIUI, the Ok() case will never be matched using a timeout of 0.

I don't grok what the expected flow is supposed be here, but hopefully this will narrow down the underlying cause.

diff --git a/vulkano/src/sync/future/fence_signal.rs b/vulkano/src/sync/future/fence_signal.rs
index cf55be79..c826821f 100644
--- a/vulkano/src/sync/future/fence_signal.rs
+++ b/vulkano/src/sync/future/fence_signal.rs
@@ -164,7 +164,7 @@ impl<F> FenceSignalFuture<F>
 
         match *state {
             FenceSignalFutureState::Flushed(ref mut prev, ref fence) => {
-                match fence.wait(Some(Duration::from_secs(0))) {
+                match fence.wait(Some(Duration::from_millis(18))) {
                     Ok(()) => unsafe {
                         prev.signal_finished()
                     },

ggazebo avatar Mar 09 '18 19:03 ggazebo

@ggazebo you might be interested in the solution mentioned in this comment - it's similar to the solution you mention but uses the existing FenceSignalFuture::wait method on the future returned by .then_signal_fence_and_flush() method and avoids the need to hack on vulkano internals.

mitchmindtree avatar Jan 02 '19 05:01 mitchmindtree

CpuBufferPool no longer exists, so this is probably irrelevant now.

Rua avatar Jan 12 '23 13:01 Rua