VIDEO/THREAD: Add idle barrier and harden wrapper API for driver deinit

Primary fix: idle barrier before driver deinit
==============================================

When the video driver is torn down (driver reinit from fullscreen
toggle, driver switch, or core launch, or full shutdown), the
deinit sequence in driver_uninit() runs on the main thread:

  1) gfx_widgets_deinit()        -- frees widget GPU resources
  2) menu_driver_ctl(DEINIT)     -- calls xmb_context_destroy /
                                    ozone_context_destroy /
                                    materialui_context_destroy,
                                    which free textures and fonts
  3) video_driver_free_internal()-- finally stops the video thread
                                    via sthread_join

Between steps 1 and 3, the video thread is still alive and may
be processing a frame that was queued before the deinit began.
That in-flight frame can reference textures and fonts that step 1
or 2 has just freed, producing use-after-free crashes -- most
visibly on D3D12 and Vulkan where the GPU driver validates
texture handles.

A comment at step 1 already acknowledges the issue ("This
absolutely has to be done before video_driver_free_internal()")
but relies on ordering rather than synchronisation.  Prior
commits in this series added context_generation guards to each
menu driver as defensive mitigation (2ac0afa), but those only
cover the menu's own draw calls, not widgets or the video
thread's internal state.

Fix: add a true barrier in the threaded video wrapper that waits
for the video thread to finish any pending frame and become idle
(blocked on cond_thread waiting for the next command).  The
barrier uses the existing thr->lock mutex and thr->cond_cmd
signal -- the frame processing loop already signals cond_cmd
after clearing thr->frame.updated (line 509 of the frame loop),
so the barrier just waits for the frame.updated flag to clear.
Secondary fix: harden wrapper API against reinit window misuse
==============================================================

A comment in video_driver_texture_load() acknowledges a latent
misuse window: "During reinit, video_st->threaded may already
reflect the new setting while video_st->data still points to
the real driver, not thread_video_t."  That site was previously
fixed by requiring both VIDEO_DRIVER_IS_THREADED_INTERNAL and
VIDEO_FLAG_THREAD_WRAPPER_ACTIVE before taking the threaded
path.  But the underlying wrapper functions themselves still
unconditionally cast video_st->data to thread_video_t*, making
them unsafe if called from any other path that only checks the
configuration flag.

The three public wrapper functions that read video_st->data
are now self-safe: each checks VIDEO_FLAG_THREAD_WRAPPER_ACTIVE
as the first step before casting.

  - video_thread_wait_idle():    returns (no-op, matches intent)
  - video_thread_font_init():    returns false (no-op)
  - video_thread_texture_handle(): falls back to func(data)
    on the caller's thread -- matches the existing "already
    on video thread" branch semantics, preserving texture
    loads during any reinit-window call.

For video_thread_font_init specifically, this closes a
pre-existing latent bug: its sole caller in font_driver.c
guards on `is_threaded` (the configuration flag) only, not on
VIDEO_FLAG_THREAD_WRAPPER_ACTIVE.  During a reinit transition
where those two disagree, the function would previously cast
a raw driver handle to thread_video_t* and access thr->lock,
producing undefined behaviour.

Changes
=======

- gfx/video_thread_wrapper.c:
  * Add video_thread_wait_idle() -- the barrier.  Guarded on
    VIDEO_FLAG_THREAD_WRAPPER_ACTIVE, then checks thr, then
    checks for self-deadlock (called from video thread), then
   waits on cond_cmd while frame.updated is true.
  * Harden video_thread_font_init with the flag check.
  * Harden video_thread_texture_handle with the flag check,
    falling back to direct func invocation.

- gfx/video_thread_wrapper.h: export video_thread_wait_idle().

- retroarch.c: call video_thread_wait_idle() at the top of
  driver_uninit(), before gfx_widgets_deinit and menu_driver_ctl
  (DEINIT).  Guarded by HAVE_THREADS, DRIVERS_VIDEO_INPUT flag,
  VIDEO_DRIVER_IS_THREADED_INTERNAL, and
  VIDEO_FLAG_THREAD_WRAPPER_ACTIVE to be a no-op when not
  applicable.

Cost
====

The barrier adds one lock acquire + one potential cond_wait per
driver reinit.  Common case (no pending frame) returns
immediately.  Worst case blocks for up to one frame duration
(~16ms) during a driver reinit which already pauses rendering.

The hardening adds one flag read per call to the three public
wrapper functions.  No measurable overhead.

Compatibility
=============

Non-threaded video: all new code paths are short-circuited by
the flag checks.  Functionally invisible.

Builds without HAVE_THREADS: the entire barrier call site in
retroarch.c is #ifdef-guarded, and the wrapper file is not
compiled.  Zero footprint.

Defense in depth: the context_generation guards from 2ac0afa
remain.  With this barrier in place, they should never fire
in practice during normal driver reinit.

No behavioural change during normal operation.  Eliminates the
TOCTOU window between menu GPU resource freeing and video
thread shutdown, and closes a pre-existing reinit-window bug
in video_thread_font_init.
This commit is contained in:
U-DESKTOP-SPFP6AQ\twistedtechre committed 2026-04-16 13:24:00 +02:00
1 parent 303168b6fe
commit 8d55232dbb
3 files changed
+84 -2

No files matched your search

+60 -2
View File
@@ -1441,7 +1441,17 @@ bool video_thread_font_init(const void **font_driver, void **font_handle,
{
thread_packet_t pkt;
video_driver_state_t *video_st = video_state_get_ptr();
thread_video_t *thr = (thread_video_t*)video_st->data;
thread_video_t *thr;
/* Only safe to interpret video_st->data as a thread_video_t*
* when the threaded video wrapper is actually active. During
* driver reinit, is_threaded may already reflect the new
* configuration while video_st->data still points to the
* previous (possibly non-threaded) driver's private state. */
if (!(video_st->flags & VIDEO_FLAG_THREAD_WRAPPER_ACTIVE))
return false;
thr = (thread_video_t*)video_st->data;
if (!thr)
return false;
@@ -1465,7 +1475,19 @@ unsigned video_thread_texture_handle(void *data, custom_command_method_t func)
{
thread_packet_t pkt;
video_driver_state_t *video_st = video_state_get_ptr();
thread_video_t *thr = (thread_video_t*)video_st->data;
thread_video_t *thr;
/* Only safe to interpret video_st->data as a thread_video_t*
* when the threaded video wrapper is actually active. During
* driver reinit, callers' "threaded" flags may already reflect
* the new configuration while video_st->data still points to
* the previous driver's private state. Fall back to calling
* func directly (same contract as the "already on video
* thread" branch below). */
if (!(video_st->flags & VIDEO_FLAG_THREAD_WRAPPER_ACTIVE))
return func(data);
thr = (thread_video_t*)video_st->data;
if (!thr)
return 0;
@@ -1483,3 +1505,39 @@ unsigned video_thread_texture_handle(void *data, custom_command_method_t func)
return pkt.data.custom_command.return_value;
}
/* Waits until the video thread has finished processing any
* pending frame and is idle, waiting on its command condition
* variable. After this returns, it is safe to free GPU-backed
* resources (textures, fonts) owned by the menu driver — no
* frame can be in-flight referencing them.
*
* Must be called from the main thread. No-op if the video
* thread is not running or if called from the video thread
* itself (would deadlock). */
void video_thread_wait_idle(void)
{
video_driver_state_t *video_st = video_state_get_ptr();
thread_video_t *thr;
/* Only safe to interpret video_st->data as a thread_video_t*
* when the threaded video wrapper is actually active. With
* non-threaded video, video_st->data points to the raw
* driver's private state. */
if (!(video_st->flags & VIDEO_FLAG_THREAD_WRAPPER_ACTIVE))
return;
thr = (thread_video_t*)video_st->data;
if (!thr || !thr->thread)
return;
/* Avoid self-deadlock if called from the video thread. */
if (sthread_get_thread_id(thr->thread) == sthread_get_current_thread_id())
return;
slock_lock(thr->lock);
while (thr->frame.updated)
scond_wait(thr->cond_cmd, thr->lock);
slock_unlock(thr->lock);
}
+6
View File
@@ -275,6 +275,12 @@ bool video_thread_font_init(
unsigned video_thread_texture_handle(void *data,
custom_command_method_t func);
/* Barrier: wait until the video thread is idle (no pending frame).
* Must be called from the main thread before freeing GPU resources
* that an in-flight frame might reference. No-op on non-threaded
* video or when called from the video thread. */
void video_thread_wait_idle(void);
RETRO_END_DECLS
#endif
+18
View File
@@ -194,6 +194,9 @@
#include "gfx/video_driver.h"
#include "gfx/video_display_server.h"
#ifdef HAVE_THREADS
#include "gfx/video_thread_wrapper.h"
#endif
#ifdef HAVE_BLUETOOTH
#include "bluetooth/bluetooth_driver.h"
#endif
@@ -1818,6 +1821,21 @@ void driver_uninit(int flags, enum driver_lifetime_flags lifetime_flags)
core_info_deinit_list();
core_info_free_current_core();
#ifdef HAVE_THREADS
/* Wait for the video thread to finish any pending frame before
* freeing GPU resources owned by the menu driver and widgets.
* Without this barrier, the video thread can be mid-render on
* frame N-1 while the main thread frees textures/fonts that
* the in-flight frame still references, producing
* use-after-free crashes on D3D12/Vulkan under threaded video.
*
* No-op when threaded video is not active. */
if ( (flags & DRIVERS_VIDEO_INPUT)
&& VIDEO_DRIVER_IS_THREADED_INTERNAL(video_st)
&& (video_st->flags & VIDEO_FLAG_THREAD_WRAPPER_ACTIVE))
video_thread_wait_idle();
#endif
#if defined(HAVE_GFX_WIDGETS)
/* This absolutely has to be done before video_driver_free_internal()
* is called/completes, otherwise certain menu drivers