games-on-whales/gst-wayland-display

Fork 0

mirror of https://github.com/games-on-whales/gst-wayland-display.git synced 2026-10-15 12:51:57 +00:00

[PR #67] [CLOSED] fix(cuda): give each CUDAContext wrapper its own reference #66

New issue

Closed

opened 2026-10-15 12:48:05 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 12:48:05 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/games-on-whales/gst-wayland-display/pull/67
Author: @xerhab
Created: 10/14/2026
Status: ❌ Closed

Base: stable ← Head: fix/cuda-context-slot-ownership

📝 Commits (1)

58c55f7 fix(cuda): give each CUDAContext wrapper its own reference

📊 Changes

2 files changed (+149 additions, -98 deletions)

View changed files

📝 gst-plugin-wayland-display/src/waylandsrc/imp.rs (+8 -13)
📝 wayland-display-core/src/utils/allocator/cuda/mod.rs (+141 -85)

📄 Description

Summary: An application that hands waylanddisplaysrc one shared CUDA context for every pipeline (as Wolf does) no longer loses a reference to it per pipeline, so streams stop freezing after a few reconnects.

Why

Answering NEED_CONTEXT with a context the application keeps loses one reference per pipeline. Wolf caches one (app_state->gst_context), so after a few streams it is finalized under a live pipeline: gst_cuda_context_push: assertion 'GST_IS_CUDA_CONTEXT (ctx)' failed, Cached CUDA context appears to be invalid (push failed), gst_object_unref: assertion 'ref_count > 0' failed, then the next stream's encoder freezes.

Root cause: the answer re-enters set_context() while new_from_gstreamer() is still inside gst_cuda_ensure_element_context(). The inner new_from_set_context() fills the slot with the one reference gst_cuda_handle_set_context() transfers and stores a wrapper owning it. The outer call sees the slot go from NULL to set, treats that as a transfer to itself, and builds and drops a second wrapper owning the same reference. adopt_slot_ref() (#42) handles the helper creating the context, where the slot is already set at re-entry; the slot's before/after values cannot tell the two apart.

What changed

The slot (CudaContextSlot) owns the helpers' (transfer full) reference and releases it with the element.
Every CUDAContext built from it takes its own reference (from_slot), so no call infers a transfer. Replaces adopt_slot_ref().
The slot moved out of Settings onto the element, since the helpers write it while re-entering set_context(), which takes the Settings lock.

Risk

Low. Only the CUDA context acquisition in waylanddisplaysrc changes. A mistake would show as a leaked context per element (memory) or as the same over-release. Both paths were checked with refcounts and the leaks tracer (below). Revert this commit to roll back.

Testing

QA result: PASS — refcounts checked on a GPU for both context sources (application-provided and self-created); regression test proven to fail on the old rule.
Not tested: a full Wolf build with this commit (tested with the plugin built against GStreamer 1.26.7 from Wolf's base image); AMD/Intel paths (untouched, cuda feature only).

Test evidence

New test_application_context_survives_pipelines: answers NEED_CONTEXT from a bus sync handler that re-enters new_from_set_context() on the same slot, three times, and checks the application's context returns to its baseline refcount. With the old ownership rule restored it fails (left: 1, right: 2).
C program mimicking Wolf (one cached GstCudaContext handed to six waylanddisplaysrc ! fakesink pipelines in turn), RTX 3090, GStreamer 1.26.7. Before: ref_count 2 → 1 → 0 (finalized, invalid unclassed pointer CRITICALs) by the 2nd pipeline. After: 2 after every pipeline.
Same program with no application context (the element creates its own): no crash; the leaks tracer reports nothing alive after gst_deinit(), before and after.
cargo fmt --all --check clean. test_cuda_buffer fails on hosts without /dev/dri/renderD129 (hardcoded node), with or without this change.

Follow-ups

test_cuda_buffer hardcodes /dev/dri/renderD129 — not changed here.

_{🔄 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/67 **Author:** [@xerhab](https://github.com/xerhab) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `stable` ← **Head:** `fix/cuda-context-slot-ownership` --- ### 📝 Commits (1) - [`58c55f7`](https://github.com/games-on-whales/gst-wayland-display/commit/58c55f7d9b9466c2d8b0b78a5d32bf6465fc2189) fix(cuda): give each CUDAContext wrapper its own reference ### 📊 Changes **2 files changed** (+149 additions, -98 deletions) <details> <summary>View changed files</summary> 📝 `gst-plugin-wayland-display/src/waylandsrc/imp.rs` (+8 -13) 📝 `wayland-display-core/src/utils/allocator/cuda/mod.rs` (+141 -85) </details> ### 📄 Description **Summary:** An application that hands `waylanddisplaysrc` one shared CUDA context for every pipeline (as Wolf does) no longer loses a reference to it per pipeline, so streams stop freezing after a few reconnects. ## Why Answering NEED_CONTEXT with a context the application keeps loses one reference per pipeline. Wolf caches one (`app_state->gst_context`), so after a few streams it is finalized under a live pipeline: `gst_cuda_context_push: assertion 'GST_IS_CUDA_CONTEXT (ctx)' failed`, `Cached CUDA context appears to be invalid (push failed)`, `gst_object_unref: assertion 'ref_count > 0' failed`, then the next stream's encoder freezes. Root cause: the answer re-enters `set_context()` while `new_from_gstreamer()` is still inside `gst_cuda_ensure_element_context()`. The inner `new_from_set_context()` fills the slot with the one reference `gst_cuda_handle_set_context()` transfers and stores a wrapper owning it. The outer call sees the slot go from NULL to set, treats that as a transfer to itself, and builds and drops a second wrapper owning the same reference. `adopt_slot_ref()` (#42) handles the helper *creating* the context, where the slot is already set at re-entry; the slot's before/after values cannot tell the two apart. ## What changed - The slot (`CudaContextSlot`) owns the helpers' `(transfer full)` reference and releases it with the element. - Every `CUDAContext` built from it takes its own reference (`from_slot`), so no call infers a transfer. Replaces `adopt_slot_ref()`. - The slot moved out of `Settings` onto the element, since the helpers write it while re-entering `set_context()`, which takes the `Settings` lock. ## Risk Low. Only the CUDA context acquisition in `waylanddisplaysrc` changes. A mistake would show as a leaked context per element (memory) or as the same over-release. Both paths were checked with refcounts and the leaks tracer (below). Revert this commit to roll back. ## Testing **QA result:** PASS — refcounts checked on a GPU for both context sources (application-provided and self-created); regression test proven to fail on the old rule. **Not tested:** a full Wolf build with this commit (tested with the plugin built against GStreamer 1.26.7 from Wolf's base image); AMD/Intel paths (untouched, `cuda` feature only). <details><summary>Test evidence</summary> - New `test_application_context_survives_pipelines`: answers NEED_CONTEXT from a bus sync handler that re-enters `new_from_set_context()` on the same slot, three times, and checks the application's context returns to its baseline refcount. With the old ownership rule restored it fails (`left: 1, right: 2`). - C program mimicking Wolf (one cached `GstCudaContext` handed to six `waylanddisplaysrc ! fakesink` pipelines in turn), RTX 3090, GStreamer 1.26.7. Before: ref_count 2 → 1 → 0 (finalized, `invalid unclassed pointer` CRITICALs) by the 2nd pipeline. After: 2 after every pipeline. - Same program with no application context (the element creates its own): no crash; the leaks tracer reports nothing alive after `gst_deinit()`, before and after. - `cargo fmt --all --check` clean. `test_cuda_buffer` fails on hosts without `/dev/dri/renderD129` (hardcoded node), with or without this change. </details> ## Follow-ups - `test_cuda_buffer` hardcodes `/dev/dri/renderD129` — not changed here. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-15 12:48:05 +00:00

closed this issue
added the
pull-request
label

No labels

enhancement

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

games-on-whales/gst-wayland-display#66

No description provided.

Rows
Columns