Skip to content

fix(linux/pipewire): avoid memory leaks#5360

Open
Kishi85 wants to merge 4 commits into
LizardByte:masterfrom
Kishi85:kwin-explict-resource-cleanup
Open

fix(linux/pipewire): avoid memory leaks#5360
Kishi85 wants to merge 4 commits into
LizardByte:masterfrom
Kishi85:kwin-explict-resource-cleanup

Conversation

@Kishi85

@Kishi85 Kishi85 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Description

In #5346 a memory leak in pipewire-based capture methods (portalgrab, kwingrab) was identified.

This PR is a fixes (potential) causes in pipewire-based capture by:

  • Changing dummy_img() to just return 0 without allocating memory as that is enough to mark a dummy image (same logic as kmsgrab/wlgrab)
  • Override the destructor of the used egl::img_descriptor_t to free allocated memory for the pipewire specific alloc_image() override by using a new subclass (also like kmsgrab/wlgrab).
  • Explicitly release any memory resources from kwingrab by calling reset() on any created shared_ptr instances in screencast_t and clear() on used lists/maps in screencast_t during destruction.
  • Cleanup pipewire_t destruction sequence and roll cleanup_stream in as is is never used outside of it.

Screenshot

Issues Fixed or Closed

Roadmap Issues

Type of Change

  • feat: New feature (non-breaking change which adds functionality)
  • fix: Bug fix (non-breaking change which fixes an issue)
  • docs: Documentation only changes
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semicolons, etc.)
  • refactor: Code change that neither fixes a bug nor adds a feature
  • perf: Code change that improves performance
  • test: Adding missing tests or correcting existing tests
  • build: Changes that affect the build system or external dependencies
  • ci: Changes to CI configuration files and scripts
  • chore: Other changes that don't modify src or test files
  • revert: Reverts a previous commit
  • BREAKING CHANGE: Introduces a breaking change (can be combined with any type above)

Checklist

  • Code follows the style guidelines of this project
  • Code has been self-reviewed
  • Code has been commented, particularly in hard-to-understand areas
  • Code docstring/documentation-blocks for new or existing methods/components have been added or updated
  • Unit tests have been added or updated for any new or modified functionality

AI Usage

  • None: No AI tools were used in creating this PR
  • Light: AI provided minor assistance (formatting, simple suggestions)
  • Moderate: AI helped with code generation or debugging specific parts
  • Heavy: AI generated most or all of the code changes

@Kishi85

Kishi85 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

In theory this PR shouldn't make a difference as C++ should manage memory for those types (std::shared_ptr, std::map, std::vector) by itself correctly for how they are used in kwingrab.

@psyke83 maybe you can check if it makes a difference on your end as my tests are a bit inconclusive right now...

@Kishi85 Kishi85 marked this pull request as draft June 29, 2026 14:29
@Kishi85 Kishi85 force-pushed the kwin-explict-resource-cleanup branch from d839b36 to 285f5e1 Compare June 30, 2026 12:55
@Kishi85

Kishi85 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Doing more testing and also cleaning up pipewire's destructor a bit but so far no luck with the memory leak. It's almost a stable ~16MB increase per full capture re-init for both kwingrab and portalgrab so the culprit is likely somewhere in pipewire.cpp or related to the pipewire code used overall.

Almost forgot: If I can't find the source of the leak myself I'll change this PR to do some code maintenance (like rolling 'cleanup_stream()` into the destructor as it's not used anywhere else anymore und updating a few unnecessary things based on CLion/Sonar suggestions).

@Kishi85

Kishi85 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Doing more testing and also cleaning up pipewire's destructor a bit but so far no luck with the memory leak. It's almost a stable ~16MB increase per full capture re-init for both kwingrab and portalgrab so the culprit is likely somewhere in pipewire.cpp or related to the pipewire code used overall.

Here's another interesting finding: The leak is only ~8MB per full capture re-init for both kwingrab and portalgrab when using VAAPI compared to ~16MB when using Vulkan encoding.

@psyke83

psyke83 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

This is part of the puzzle:

diff --git a/src/platform/linux/graphics.h b/src/platform/linux/graphics.h
index 2a2a0642..60be723f 100644
--- a/src/platform/linux/graphics.h
+++ b/src/platform/linux/graphics.h
@@ -603,7 +603,11 @@ namespace egl {
   class img_descriptor_t: public cursor_t {
   public:
     ~img_descriptor_t() {
+      BOOST_LOG(error) << "[leak] reset img_descriptor_t";
       reset();
+
+      delete[] data;
+      data = nullptr;
     }
 
     /**

With your PR branch applied, this resolves leaking with vaapi combined with kwin or portal capture. Vulkan still leaks, and software rendering breaks with the change as-is (haven't looked too deeply yet).

Some capture methods employ an img_t override that does extra cleanup. Example:

struct img_t: public platf::img_t {
/**
* @brief Destroy the Wayland capture image.
*/
~img_t() override {
delete[] data;
data = nullptr;
}
};
or
struct kms_img_t: public img_t {
~kms_img_t() override {
delete[] data;
data = nullptr;
}
};

In the case of Pipewire capture, alloc_img() is overridden to use the img_descriptor_t type, but we don't do any special cleanup via a destructor override, and the standard destructor in graphics.h only cleans up the sd fds.

@Kishi85

Kishi85 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

This is part of the puzzle:

diff --git a/src/platform/linux/graphics.h b/src/platform/linux/graphics.h
index 2a2a0642..60be723f 100644
--- a/src/platform/linux/graphics.h
+++ b/src/platform/linux/graphics.h
@@ -603,7 +603,11 @@ namespace egl {
   class img_descriptor_t: public cursor_t {
   public:
     ~img_descriptor_t() {
+      BOOST_LOG(error) << "[leak] reset img_descriptor_t";
       reset();
+
+      delete[] data;
+      data = nullptr;
     }
 
     /**

With your PR branch applied, this resolves leaking with vaapi combined with kwin or portal capture. Vulkan still leaks, and software rendering breaks with the change as-is (haven't looked too deeply yet).

Some capture methods employ an img_t override that does extra cleanup. Example:

struct img_t: public platf::img_t {
/**
* @brief Destroy the Wayland capture image.
*/
~img_t() override {
delete[] data;
data = nullptr;
}
};

or

struct kms_img_t: public img_t {
~kms_img_t() override {
delete[] data;
data = nullptr;
}
};

In the case of Pipewire capture, alloc_img() is overridden to use the img_descriptor_t type, but we don't do any special cleanup via a destructor override, and the standard destructor in graphics.h only cleans up the sd fds.

So we probably have to just add a subclassed img_descriptor_t with the extra cleanup for pipewire and use that in the override?

Also is there a specific reason for pipewire using egl::img_descriptor_t instead of platf::img_t (like kms/wlgrab)?

@psyke83

psyke83 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

I'm not sure why that was done, but more investigation actually points this specific leak issue to be isolated to dummy_img:

int dummy_img(platf::img_t *img) override {
if (!img) {
return -1;
}
img->data = new std::uint8_t[img->height * img->row_pitch];
std::fill_n(img->data, img->height * img->row_pitch, 0);
return 0;
}

Reverting my previous changes and instead short-circuiting dummy_img (inserting return 0; at the start or just deleting the function contents) also resolves the memory leak with the vaapi encoder, and software encoding no longer crashes (and also doesn't leak). Only Vulkan leaks as-is, which I assume is a separate unrelated leak.

Edit: I won't be able to test thoroughly until later, but if it turns out that the destructor really does need to do cleanup, being able to distinguish the ownership between dummy_img()'s vs other img_descriptor_t allocations would probably resolve the double-free crashing. Something like the below snippet. But perhaps there's a more appropriate fix.

diff --git a/src/platform/linux/graphics.h b/src/platform/linux/graphics.h
index 2a2a0642..997ee0f2 100644
--- a/src/platform/linux/graphics.h
+++ b/src/platform/linux/graphics.h
@@ -617,6 +617,11 @@ namespace egl {
           sd.fds[x] = -1;
         }
       }
+      if (data && data_allocated) {
+        delete[] data;
+      }
+      data = nullptr;
+      data_allocated = false;
     }
 
     surface_descriptor_t sd;  ///< DMA-BUF surface descriptor for the captured image.
@@ -632,6 +637,8 @@ namespace egl {
     std::optional<uint64_t> seq;  ///< PipeWire frame sequence number.
     std::optional<bool> pw_damage;  ///< Whether PipeWire damage tracking should be used.
     std::optional<uint32_t> pw_flags;  ///< PipeWire frame flags reported with the buffer.
+
+    bool data_allocated {false};
   };
 
   /**
diff --git a/src/platform/linux/pipewire.cpp b/src/platform/linux/pipewire.cpp
index fe71ff3b..7f53d6a4 100644
--- a/src/platform/linux/pipewire.cpp
+++ b/src/platform/linux/pipewire.cpp
@@ -1055,6 +1055,7 @@ namespace pipewire {
 
       img->data = new std::uint8_t[img->height * img->row_pitch];
       std::fill_n(img->data, img->height * img->row_pitch, 0);
+      static_cast<egl::img_descriptor_t*>(img)->data_allocated = true;
       return 0;
     }

@Kishi85

Kishi85 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

I'm not sure why that was done, but more investigation actually points this specific leak issue to be isolated to dummy_img:

int dummy_img(platf::img_t *img) override {
if (!img) {
return -1;
}
img->data = new std::uint8_t[img->height * img->row_pitch];
std::fill_n(img->data, img->height * img->row_pitch, 0);
return 0;
}

Reverting my previous changes and instead short-circuiting dummy_img (inserting return 0; at the start or just deleting the function contents) also resolves the memory leak with the vaapi encoder, and software encoding no longer crashes (and also doesn't leak). Only Vulkan leaks as-is, which I assume is a separate unrelated leak.

Edit: I won't be able to test thoroughly until later, but if it turns out that the destructor really does need to do cleanup, being able to distinguish the ownership between dummy_img()'s vs other img_descriptor_t allocations would probably resolve the double-free crashing. Something like the below snippet. But perhaps there's a more appropriate fix.

diff --git a/src/platform/linux/graphics.h b/src/platform/linux/graphics.h
index 2a2a0642..997ee0f2 100644
--- a/src/platform/linux/graphics.h
+++ b/src/platform/linux/graphics.h
@@ -617,6 +617,11 @@ namespace egl {
           sd.fds[x] = -1;
         }
       }
+      if (data && data_allocated) {
+        delete[] data;
+      }
+      data = nullptr;
+      data_allocated = false;
     }
 
     surface_descriptor_t sd;  ///< DMA-BUF surface descriptor for the captured image.
@@ -632,6 +637,8 @@ namespace egl {
     std::optional<uint64_t> seq;  ///< PipeWire frame sequence number.
     std::optional<bool> pw_damage;  ///< Whether PipeWire damage tracking should be used.
     std::optional<uint32_t> pw_flags;  ///< PipeWire frame flags reported with the buffer.
+
+    bool data_allocated {false};
   };
 
   /**
diff --git a/src/platform/linux/pipewire.cpp b/src/platform/linux/pipewire.cpp
index fe71ff3b..7f53d6a4 100644
--- a/src/platform/linux/pipewire.cpp
+++ b/src/platform/linux/pipewire.cpp
@@ -1055,6 +1055,7 @@ namespace pipewire {
 
       img->data = new std::uint8_t[img->height * img->row_pitch];
       std::fill_n(img->data, img->height * img->row_pitch, 0);
+      static_cast<egl::img_descriptor_t*>(img)->data_allocated = true;
       return 0;
     }

Both kmsgrab and wlgrab implement dummy_img() by just returning 0:

* @brief Populate a fallback image when real capture data is unavailable.
*
* @param img Image or frame object to read from or populate.
* @return Capture status reported to the streaming pipeline.
*/
int dummy_img(platf::img_t *img) override {
return 0;
}

/**
* @brief Populate a fallback image when real capture data is unavailable.
*
* @param img Image or frame object to read from or populate.
* @return Capture status reported to the streaming pipeline.
*/
int dummy_img(platf::img_t *img) override {
// Empty images are recognized as dummies by the zero sequence number
return 0;
}

/**
* @brief Populate a fallback image when real capture data is unavailable.
*
* @param img Image or frame object to read from or populate.
* @return Capture status reported to the streaming pipeline.
*/
int dummy_img(platf::img_t *img) override {
return 0;
}

So I'd say that pipewire.cpp should just do the same thing to keep things similar between linux capture methods. There seems to be no functional disadvantage from doing it this way.

@Kishi85 Kishi85 force-pushed the kwin-explict-resource-cleanup branch 2 times, most recently from 5d88955 to 6a2e3b9 Compare July 2, 2026 10:25
@Kishi85

Kishi85 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@psyke83 I think you've found the main issues here. To keep things simple and in line with kmsgrab/wlgrab I've just added a subclass for those specific img_descriptor_t's. Also updated dummy_img() to what kmsgrab/wlgrab do and with that I'm not seeing any obvious memory leaks anymore using vaapi.

There might still be a small leak occurring but that would be somewhere affecting all capture methods including kmsgrab/wlgrab and a separate issue.

I also can confirm the memory leak in vulkan encoding as that is still happening albeit at half the previous increase (which was somewhat to be expected due to given behaviour). Update: using KMS also has the same 8MB per capture re-init memory increase using vulkan, so definitely a separate issue that is vulkan specific.

I'll update the PR and mark this as ready unless you have something else to add.

@Kishi85 Kishi85 force-pushed the kwin-explict-resource-cleanup branch from 6a2e3b9 to 8d011b5 Compare July 2, 2026 10:35
@Kishi85 Kishi85 changed the title fix(linux/kwin): explicitly cleanup screencast_t allocated memory resources to ensure nothing is leaking fix(linux/pipewire): avoid memory leaks Jul 2, 2026
@Kishi85 Kishi85 marked this pull request as ready for review July 2, 2026 10:50
@Kishi85 Kishi85 force-pushed the kwin-explict-resource-cleanup branch from 8d011b5 to dae4b14 Compare July 2, 2026 14:53
@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 New issue
1 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@psyke83

psyke83 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

I don't think the descriptor cleanup is necessary/sensible when dummy_img() becomes a no-op?

diff --git a/src/platform/linux/pipewire.cpp b/src/platform/linux/pipewire.cpp
index 1048b839..e5c133d4 100644
--- a/src/platform/linux/pipewire.cpp
+++ b/src/platform/linux/pipewire.cpp
@@ -129,6 +129,7 @@ namespace pipewire {
   struct img_descriptor_t: public egl::img_descriptor_t {
     ~img_descriptor_t() override {
       if (data) {
+        BOOST_LOG(warning) << "[leak]: deleting data";
         delete[] data;
         data = nullptr;
       }

This log is not appearing with vaapi or software encoding on any of the pipewire capture methods, and doesn't seem to affect memory usage (naturally). Also, I should've clarified "no leak" to include the caveat that I'm ignoring the first reinit cycle. There usually seems to be an increase after first reinit, but the usage stays consistent on subsequent reinit cycles.

@Kishi85

Kishi85 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

I don't think the descriptor cleanup is necessary/sensible when dummy_img() becomes a no-op?

diff --git a/src/platform/linux/pipewire.cpp b/src/platform/linux/pipewire.cpp
index 1048b839..e5c133d4 100644
--- a/src/platform/linux/pipewire.cpp
+++ b/src/platform/linux/pipewire.cpp
@@ -129,6 +129,7 @@ namespace pipewire {
   struct img_descriptor_t: public egl::img_descriptor_t {
     ~img_descriptor_t() override {
       if (data) {
+        BOOST_LOG(warning) << "[leak]: deleting data";
         delete[] data;
         data = nullptr;
       }

This log is not appearing with vaapi or software encoding on any of the pipewire capture methods, and doesn't seem to affect memory usage (naturally). Also, I should've clarified "no leak" to include the caveat that I'm ignoring the first reinit cycle. There usually seems to be an increase after first reinit, but the usage stays consistent on subsequent reinit cycles.

I've not tried that logging but looking at the debugger it's always NULL for the cases I've seen so far, so it's probably not strictly necessary. I'd still leave it in place for consistency (ensuring that the img_descriptor data is always properly cleared) with other capture methods (namely kmsgrab/wlgrab).

@psyke83

psyke83 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

I believe that the Vulkan issue should be kept separate from this PR, as there's a leak in the n8.1.2 build from Arch which should roughly match our current build revision?

Reproduction of leak from command-line invocation (needs vulkan-validation-layers installed):

$ VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation \
ffmpeg -y \
  -init_hw_device vulkan=vk \
  -filter_hw_device vk \
  -f lavfi -i testsrc=size=1280x720:rate=30 \
  -frames:v 1 \
  -vf format=nv12,hwupload \
  -c:v h264_vulkan \
  out.h264 2> validation_repro.log
Validation Error: [ VUID-VkImageMemoryBarrier2-srcAccessMask-03915 ] | MessageID = 0xb927d7cf
vkCmdPipelineBarrier2(): pDependencyInfo->pImageMemoryBarriers[0].srcAccessMask (VK_ACCESS_2_TRANSFER_WRITE_BIT) is not supported by stage mask (VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT). The expansion of VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT for VK_QUEUE_VIDEO_ENCODE_BIT_KHR does not include any stages that support VK_ACCESS_2_TRANSFER_WRITE_BIT.

The expanded stages are: VK_PIPELINE_STAGE_2_TOP_OF_PIPE_BIT|VK_PIPELINE_STAGE_2_BOTTOM_OF_PIPE_BIT|VK_PIPELINE_STAGE_2_VIDEO_ENCODE_BIT_KHR.
The Vulkan spec states: If srcAccessMask includes VK_ACCESS_2_TRANSFER_WRITE_BIT, srcStageMask must include VK_PIPELINE_STAGE_2_COPY_BIT, VK_PIPELINE_STAGE_2_BLIT_BIT, VK_PIPELINE_STAGE_2_RESOLVE_BIT, VK_PIPELINE_STAGE_2_CLEAR_BIT, VK_PIPELINE_STAGE_2_ALL_TRANSFER_BIT, VK_PIPELINE_STAGE_2_ACCELERATION_STRUCTURE_BUILD_BIT_KHR, VK_PIPELINE_STAGE_2_ACCELERATION_STRUCTURE_COPY_BIT_KHR, VK_PIPELINE_STAGE_2_CONVERT_COOPERATIVE_VECTOR_MATRIX_BIT_NV, or VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT (https://docs.vulkan.org/spec/latest/chapters/synchronization.html#VUID-VkImageMemoryBarrier2-srcAccessMask-03915)
Objects: 1
    [0] VkCommandBuffer 0x7f319c6a0960

Validation Error: [ VUID-vkDestroyDevice-device-05137 ] | MessageID = 0x4872eaa0
vkDestroyDevice(): VkDevice 0x558956ba7ed0 has 1 leaked objects that have not been destroyed.
VkImageView 0x590000000059
The Vulkan spec states: All child objects created on device that can be destroyed or freed must have been destroyed or freed prior to destroying device (https://docs.vulkan.org/spec/latest/chapters/devsandqueues.html#VUID-vkDestroyDevice-device-05137)

I added some logging of VkImage and VkImageView creation and destruction in Sunshine and appended this to my systemd daemon config:

$ cat ~/.config/systemd/user/app-dev.lizardbyte.app.Sunshine.service.d/override.conf 
[Service]
Environment=VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation

diff --git a/src/platform/linux/vulkan_encode.cpp b/src/platform/linux/vulkan_encode.cpp
index fef11ad4..da63eafd 100644
--- a/src/platform/linux/vulkan_encode.cpp
+++ b/src/platform/linux/vulkan_encode.cpp
@@ -601,6 +601,8 @@ namespace vk {
                          << ", pitch=" << sd.pitches[0] << ", offset=" << sd.offsets[0] << ")";
         return false;
       }
+      BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                         << " | Action: create src.image= " << src.image;
 
       // Bind imported DMA-BUF memory
       VkMemoryRequirements mem_req;
@@ -623,6 +625,8 @@ namespace vk {
       if (res != VK_SUCCESS) {
         BOOST_LOG(error) << "vkAllocateMemory for DMA-BUF failed: " << res;
         vkDestroyImage(vk_dev.dev, src.image, nullptr);
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: destroy src.image= " << src.image;
         src.image = VK_NULL_HANDLE;
         return false;
       }
@@ -637,6 +641,8 @@ namespace vk {
       view_ci.components = vk_swizzle;
       view_ci.subresourceRange = {VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1};
       VK_CHECK_BOOL(vkCreateImageView(vk_dev.dev, &view_ci, nullptr, &src.view));
+      BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                         << " | Action: create src.view= " << src.view;
 
       src.mem = src_mem;
       return true;
@@ -656,6 +662,8 @@ namespace vk {
       img_ci.usage = VK_IMAGE_USAGE_SAMPLED_BIT;
       img_ci.initialLayout = VK_IMAGE_LAYOUT_PREINITIALIZED;
       VK_CHECK_BOOL(vkCreateImage(vk_dev.dev, &img_ci, nullptr, &cursor.image));
+      BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                         << " | Action: create cursor.image= " << cursor.image;
 
       VkMemoryRequirements mem_req;
       vkGetImageMemoryRequirements(vk_dev.dev, cursor.image, &mem_req);
@@ -683,6 +691,8 @@ namespace vk {
       view_ci.format = VK_FORMAT_B8G8R8A8_UNORM;
       view_ci.subresourceRange = {VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1};
       VK_CHECK_BOOL(vkCreateImageView(vk_dev.dev, &view_ci, nullptr, &cursor.view));
+      BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                         << " | Action: create cursor.view= " << cursor.view;
 
       cursor.needs_transition = true;
       descriptors_dirty = true;
@@ -691,11 +701,15 @@ namespace vk {
 
     void destroy_cursor_image() {
       if (cursor.view) {
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: destroy cursor.view= " << cursor.view;
         vkDestroyImageView(vk_dev.dev, cursor.view, nullptr);
         cursor.view = VK_NULL_HANDLE;
       }
       if (cursor.image) {
         vkDestroyImage(vk_dev.dev, cursor.image, nullptr);
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: destroy cursor.image= " << cursor.image;
         cursor.image = VK_NULL_HANDLE;
       }
       if (cursor.mem) {
@@ -729,11 +743,15 @@ namespace vk {
         view_ci.format = y_fmt;
         view_ci.subresourceRange = {VK_IMAGE_ASPECT_PLANE_0_BIT, 0, 1, 0, 1};
         VK_CHECK_BOOL(vkCreateImageView(vk_dev.dev, &view_ci, nullptr, &target.y_view));
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: create target.y_view= " << target.y_view;
 
         // UV plane
         view_ci.format = uv_fmt;
         view_ci.subresourceRange = {VK_IMAGE_ASPECT_PLANE_1_BIT, 0, 1, 0, 1};
         VK_CHECK_BOOL(vkCreateImageView(vk_dev.dev, &view_ci, nullptr, &target.uv_view));
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: create target.uv_view= " << target.uv_view;
       } else {
         // Separate images per plane
         VkImageViewCreateInfo view_ci = {VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO};
@@ -743,10 +761,14 @@ namespace vk {
         view_ci.image = vk_frame->img[0];
         view_ci.format = y_fmt;
         VK_CHECK_BOOL(vkCreateImageView(vk_dev.dev, &view_ci, nullptr, &target.y_view));
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: create target.y_view= " << target.y_view;
 
         view_ci.image = vk_frame->img[1];
         view_ci.format = uv_fmt;
         VK_CHECK_BOOL(vkCreateImageView(vk_dev.dev, &view_ci, nullptr, &target.uv_view));
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: create target.uv_view= " << target.uv_view;
       }
       return true;
     }
@@ -916,10 +938,16 @@ namespace vk {
         // By the time we wrap around (4 frames later), it's guaranteed done.
         auto &slot = defer_ring[defer_idx];
         if (slot.view) {
+          BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                             << " | Action: destroy (defer_idx slot " << defer_idx
+                             << ") slot.view= " << slot.view;
           vkDestroyImageView(vk_dev.dev, slot.view, nullptr);
         }
         if (slot.image) {
           vkDestroyImage(vk_dev.dev, slot.image, nullptr);
+          BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                             << " | Action: destroy (defer_idx slot " << defer_idx
+                             << ") slot.image= " << slot.image;
         }
         if (slot.mem) {
           vkFreeMemory(vk_dev.dev, slot.mem, nullptr);
@@ -934,25 +962,44 @@ namespace vk {
       if (!vk_dev.dev) {
         return;
       }
+
+    BOOST_LOG(warning) << "[leak-audit] Checking for orphans...";
+    BOOST_LOG(warning) << "[leak-audit] src.view: " << src.view;
+    BOOST_LOG(warning) << "[leak-audit] cursor.view: " << cursor.view;
+    BOOST_LOG(warning) << "[leak-audit] target.y_view: " << target.y_view;
+    BOOST_LOG(warning) << "[leak-audit] target.uv_view: " << target.uv_view;
+
       vkDeviceWaitIdle(vk_dev.dev);
       destroy_src_image();
       // Flush deferred destroys
+      int iter_idx = 0;
       for (auto &slot : defer_ring) {
         if (slot.view) {
+          BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                             << " | Action: destroy (iter_idx slot " << iter_idx
+                             << ") slot.view= " << slot.view;
           vkDestroyImageView(vk_dev.dev, slot.view, nullptr);
         }
         if (slot.image) {
           vkDestroyImage(vk_dev.dev, slot.image, nullptr);
+          BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                             << " | Action: destroy (iter_idx slot " << iter_idx
+                             << ") slot.image= " << slot.image;
         }
         if (slot.mem) {
           vkFreeMemory(vk_dev.dev, slot.mem, nullptr);
         }
         slot = {};
+        ++iter_idx;
       }
       if (target.y_view) {
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: destroy target.y_view= " << target.y_view;
         vkDestroyImageView(vk_dev.dev, target.y_view, nullptr);
       }
       if (target.uv_view) {
+        BOOST_LOG(warning) << "[leak] Device: " << vk_dev.dev
+                           << " | Action: destroy target.uv_view= " << target.uv_view;
         vkDestroyImageView(vk_dev.dev, target.uv_view, nullptr);
       }
       destroy_cursor_image();

My results show 1 leak on every encode probe cycle via video.cpp's validate_config() of a VkImageView address that doesn't correspond to any of any of the logged Sunshine calls. Subsequent connect/reconnect/disconnect sometimes leaks up to 3 VkImageView items in one go, but again, none correspond to what Sunshine logs.

Example (with validate_config() bypass to only test Vulkan during real connect):

[conn@archlinux sunshine]$ journalctl -b0 | grep 275148 | grep -Ei "leaked|0x5f500000005f5|0x5f600000005f6|0x5f700000005f7|leak-audit"
Jul 03 02:08:37 archlinux sunshine[275148]: [2026-07-03 02:08:37.423]: Warning: [leak-audit] Checking for orphans...
Jul 03 02:08:37 archlinux sunshine[275148]: [2026-07-03 02:08:37.423]: Warning: [leak-audit] src.view: 0x5eb00000005eb
Jul 03 02:08:37 archlinux sunshine[275148]: [2026-07-03 02:08:37.423]: Warning: [leak-audit] cursor.view: 0x650000000065
Jul 03 02:08:37 archlinux sunshine[275148]: [2026-07-03 02:08:37.423]: Warning: [leak-audit] target.y_view: 0x6a000000006a
Jul 03 02:08:37 archlinux sunshine[275148]: [2026-07-03 02:08:37.423]: Warning: [leak-audit] target.uv_view: 0x6b000000006b
Jul 03 02:08:37 archlinux sunshine[275148]: vkDestroyDevice(): VkDevice 0x7f952420a850 has 3 leaked objects that have not been destroyed.
Jul 03 02:08:37 archlinux sunshine[275148]: VkImageView 0x5f500000005f5, VkImageView 0x5f600000005f6, VkImageView 0x5f700000005f7

Hence the leak is more than likely on the ffmpeg side.

@Kishi85

Kishi85 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@psyke83 With that analysis I agree that the vulkan memory leak is very likely on the ffmpeg side, especially since you can reproduce it with just using ffmpeg itself. Would you mind putting your analysis into a separate issue so we can keep track of it?
EDIT: Could be related to https://code.ffmpeg.org/FFmpeg/FFmpeg/issues/21675

@ReenigneArcher the fixes for the pipewire memory leak in this PR should be good to merge

@ReenigneArcher

Copy link
Copy Markdown
Member

@Kishi85

Kishi85 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@Kishi85 I didn't read all of this conversation, but is it possible to address https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=5360&id=LizardByte_Sunshine&open=AZ8iXDHnzFzYS_Ua0It3&tab=code

Unfortunately not possible without touching all other capture methods (as that pointer is from platf::img_t). Even then its likley a change someone with a deep understanding of the whole process should make.

To be clear: That part isn't strictly necessary for pipewire-based capture (at least so far) but I'd keep that additional failsafe there for consistency with other linux capture methods. This part is a 1:1 adaption from what kmsgrab and wlgrab are both already doing for their implementations of platf::img_t. Same reasoning for the dummy_img()-change, which is now done the same way for pipewire-based capture and at least kmsgrab/wlgrab (haven't checked non-linux capture methods).

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Bundle Report

Bundle size has no change ✅

@ReenigneArcher

Copy link
Copy Markdown
Member

Hate to ask this, but can you force push again? There's a bug in GitHub where sometimes it doesn't allow the arch and flatpak builds to checkout the PR.

I usually just make a small change, amend the last commit. Then undo that change, and re-amend the last commit. Then force push. Not sure if there's an easier way.

@Kishi85 Kishi85 force-pushed the kwin-explict-resource-cleanup branch from dae4b14 to be9b000 Compare July 3, 2026 14:42
@Kishi85

Kishi85 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Hate to ask this, but can you force push again? There's a bug in GitHub where sometimes it doesn't allow the arch and flatpak builds to checkout the PR.

I usually just make a small change, amend the last commit. Then undo that change, and re-amend the last commit. Then force push. Not sure if there's an easier way.

No worries, let's see if rebasing and changing the commit order is enough to fix this.

Kishi85 and others added 3 commits July 3, 2026 10:45
This is the same implementation as for kmsgrab/wlgrab

Co-Authored-By: Psyke83 <psyke83@users.noreply.github.com>
Stop memory from leaking on pipewire's images like wlgrab and kmsgrab do.

Co-Authored-By: Psyke83 <psyke83@users.noreply.github.com>
@ReenigneArcher ReenigneArcher force-pushed the kwin-explict-resource-cleanup branch from be9b000 to 665aef9 Compare July 3, 2026 14:45
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@92a9230). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/platform/linux/pipewire.cpp 0.00% 14 Missing and 3 partials ⚠️
src/platform/linux/kwingrab.cpp 0.00% 5 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5360   +/-   ##
=========================================
  Coverage          ?   27.43%           
=========================================
  Files             ?      113           
  Lines             ?    25547           
  Branches          ?    11244           
=========================================
  Hits              ?     7010           
  Misses            ?    15476           
  Partials          ?     3061           
Flag Coverage Δ
Archlinux 0.00% <0.00%> (?)
FreeBSD-amd64 13.24% <0.00%> (?)
Homebrew-macos-14 21.00% <ø> (?)
Homebrew-macos-15 21.17% <ø> (?)
Homebrew-macos-26 21.27% <ø> (?)
Homebrew-ubuntu-24.04 13.00% <0.00%> (?)
Linux-AppImage 12.34% <0.00%> (?)
Windows-AMD64 15.26% <ø> (?)
Windows-ARM64 13.31% <ø> (?)
macOS-arm64 19.27% <ø> (?)
macOS-x86_64 18.76% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/platform/linux/kwingrab.cpp 0.00% <0.00%> (ø)
src/platform/linux/pipewire.cpp 0.16% <0.00%> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92a9230...665aef9. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants