[PR #47] [MERGED] fix(vulkan): replace guessed struct offsets with a header-compiled bridge; plug two leaks #47

New issue

Closed

opened 2026-10-15 12:47:54 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 12:47:54 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/games-on-whales/gst-wayland-display/pull/47
Author: @salty2011
Created: 7/26/2026
Status: ✅ Merged
Merged: 8/18/2026
Merged by: @JBailes

Base: feat/vulkan-encode ← Head: fix/vulkan-abi-bridge

📝 Commits (3)

75aef9f fix(vulkan): replace guessed struct offsets with a header-compiled bridge; plug two leaks
208897f review: gst_clear_object on barrier queue; drop drifted doc comment; restore fan-out gate rationale
aeaff0e fix(vulkan): resolve vkDestroySemaphore through gst; drop the libvulkan link

📊 Changes

6 files changed (+123 additions, -92 deletions)

View changed files

📝 Cargo.lock (+1 -0)
📝 ci/README.md (+1 -0)
📝 wayland-display-core/Cargo.toml (+1 -0)
📝 wayland-display-core/build.rs (+16 -2)
➕ wayland-display-core/src/utils/vulkan_bridge.c (+64 -0)
📝 wayland-display-core/src/utils/vulkan_share.rs (+40 -90)

📄 Description

The encode-src path reaches into GStreamer Vulkan structs by writing hardcoded byte offsets from Rust:

const BARRIER_QUEUE_OFFSET: usize = 296;      // barrier.parent.queue
const BARRIER_SEMAPHORE_OFFSET: usize = 320;  // barrier.parent.semaphore
const BARRIER_IMAGE_LAYOUT_OFFSET: usize = 368;

plus #[repr(C)] overlay structs mirroring GstVulkanInstance, GstVulkanDevice, GstVulkanQueue and GstVulkanImageMemory.

The comments record that the offsets were verified with an offsetof probe against the running headers, and they are correct today. The concern is the failure mode: if any of those structs gains or reorders a field, the result is a silent write to the wrong memory rather than a build error, and each overlay has to be re-verified by hand against every GStreamer version.

This replaces both mechanisms with a small C bridge (vulkan_bridge.c) compiled against the actual GStreamer Vulkan headers via build.rs, so field access is checked by the compiler and tracks the headers automatically. Two behavior changes ride along, both replacing guesses with failure:

raw_handles() no longer falls back to queue family 0 when select_queue(GRAPHICS) returns null. It returns None, and GsVulkanBuf::new turns that into Vulkan output failure instead of submitting on a guessed family.
recover_vk_image() returns None on a null image instead of handing back a garbage handle.

Both are intended. The layout and fan-out handling is unchanged.

Two leaks surfaced while moving the barrier reset into the bridge

Per-image VkSemaphore handle leak. Clearing barrier.parent.semaphore by writing the field left the timeline semaphore the allocator created undestroyed — one leaked handle per encode-src image. The bridge now calls vkDestroySemaphore before clearing.
Per-call GstVulkanQueue leak. gst_vulkan_device_select_queue() is transfer-full, but raw_handles() only read the queue family off the returned object and dropped the pointer, leaking one queue object per call. It is now taken with from_glib_full and released once the family index has been read.

Scope

Deliberately limited to the ABI-safety change and those two leaks. I also carry a separate change that makes the producer participate in GstVulkanQueue's submission lock, which I have not included here: it was written for a VK_ERROR_DEVICE_LOST / Xid theory on my hardware that later turned out to have a different, proven cause (a buffer-reuse gate blinded by a header copy), so I have no evidence it fixes anything and did not want to present it as a fix. Happy to open it separately if you think it is correct on principle.

Build-verified on this branch's head (43d4c25).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/games-on-whales/gst-wayland-display/pull/47 **Author:** [@salty2011](https://github.com/salty2011) **Created:** 7/26/2026 **Status:** ✅ Merged **Merged:** 8/18/2026 **Merged by:** [@JBailes](https://github.com/JBailes) **Base:** `feat/vulkan-encode` ← **Head:** `fix/vulkan-abi-bridge` --- ### 📝 Commits (3) - [`75aef9f`](https://github.com/games-on-whales/gst-wayland-display/commit/75aef9f71685fb7ff767fd98e42104b2dad69867) fix(vulkan): replace guessed struct offsets with a header-compiled bridge; plug two leaks - [`208897f`](https://github.com/games-on-whales/gst-wayland-display/commit/208897f98967223aa1de936dce9cf6243199590a) review: gst_clear_object on barrier queue; drop drifted doc comment; restore fan-out gate rationale - [`aeaff0e`](https://github.com/games-on-whales/gst-wayland-display/commit/aeaff0ea38c80714c7655046aa7fe398825566b5) fix(vulkan): resolve vkDestroySemaphore through gst; drop the libvulkan link ### 📊 Changes **6 files changed** (+123 additions, -92 deletions) <details> <summary>View changed files</summary> 📝 `Cargo.lock` (+1 -0) 📝 `ci/README.md` (+1 -0) 📝 `wayland-display-core/Cargo.toml` (+1 -0) 📝 `wayland-display-core/build.rs` (+16 -2) ➕ `wayland-display-core/src/utils/vulkan_bridge.c` (+64 -0) 📝 `wayland-display-core/src/utils/vulkan_share.rs` (+40 -90) </details> ### 📄 Description The encode-src path reaches into GStreamer Vulkan structs by writing hardcoded byte offsets from Rust: ```rust const BARRIER_QUEUE_OFFSET: usize = 296; // barrier.parent.queue const BARRIER_SEMAPHORE_OFFSET: usize = 320; // barrier.parent.semaphore const BARRIER_IMAGE_LAYOUT_OFFSET: usize = 368; ``` plus `#[repr(C)]` overlay structs mirroring `GstVulkanInstance`, `GstVulkanDevice`, `GstVulkanQueue` and `GstVulkanImageMemory`. The comments record that the offsets were verified with an `offsetof` probe against the running headers, and they are correct today. The concern is the failure mode: if any of those structs gains or reorders a field, the result is a silent write to the wrong memory rather than a build error, and each overlay has to be re-verified by hand against every GStreamer version. This replaces both mechanisms with a small C bridge (`vulkan_bridge.c`) compiled against the actual GStreamer Vulkan headers via `build.rs`, so field access is checked by the compiler and tracks the headers automatically. Two behavior changes ride along, both replacing guesses with failure: * `raw_handles()` no longer falls back to queue family 0 when `select_queue(GRAPHICS)` returns null. It returns `None`, and `GsVulkanBuf::new` turns that into Vulkan output failure instead of submitting on a guessed family. * `recover_vk_image()` returns `None` on a null image instead of handing back a garbage handle. Both are intended. The layout and fan-out handling is unchanged. ### Two leaks surfaced while moving the barrier reset into the bridge 1. **Per-image `VkSemaphore` handle leak.** Clearing `barrier.parent.semaphore` by writing the field left the timeline semaphore the allocator created undestroyed — one leaked handle per encode-src image. The bridge now calls `vkDestroySemaphore` before clearing. 2. **Per-call `GstVulkanQueue` leak.** `gst_vulkan_device_select_queue()` is transfer-full, but `raw_handles()` only read the queue family off the returned object and dropped the pointer, leaking one queue object per call. It is now taken with `from_glib_full` and released once the family index has been read. ### Scope Deliberately limited to the ABI-safety change and those two leaks. I also carry a separate change that makes the producer participate in `GstVulkanQueue`'s submission lock, which I have **not** included here: it was written for a `VK_ERROR_DEVICE_LOST` / Xid theory on my hardware that later turned out to have a different, proven cause (a buffer-reuse gate blinded by a header copy), so I have no evidence it fixes anything and did not want to present it as a fix. Happy to open it separately if you think it is correct on principle. Build-verified on this branch's head (`43d4c25`). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-15 12:47:54 +00:00