GFX/THUMBNAIL: Fix data race on thumbnail state with threaded video

gfx_thumbnail_handle_upload() runs on the main thread (via the task
queue's gather/callback path) and writes texture, width, height,
then status into a gfx_thumbnail_t.  gfx_thumbnail_draw() runs on
the video thread under threaded video and reads the same fields.

On architectures with weak memory ordering (ARM64: Switch, Android),
the status write can become visible to the video thread before the
texture/width/height writes, causing gfx_thumbnail_draw() to
attempt rendering with stale or zero values.  This results in
crashes or rendering corruption, particularly with Vulkan and D3D12
backends where invalid texture handles trigger driver-level faults.

Fix: release/acquire ordering on the status field, plus field
snapshots in the draw function.

Write side (gfx_thumbnail_handle_upload):

- Both status writes (GFX_THUMBNAIL_STATUS_MISSING at the default
  fallback path, GFX_THUMBNAIL_STATUS_AVAILABLE at the success
  path) now use GFX_THUMB_STATUS_STORE, a release-store macro.
  This ensures all prior stores (texture handle from
  video_driver_texture_load, cached width/height) are committed
  to memory before the status value becomes visible to other
  threads.

Read side (gfx_thumbnail_draw):

- The status check now uses GFX_THUMB_STATUS_LOAD, an acquire-load
  macro.  If the video thread sees AVAILABLE, all data fields
  written before the release-store are guaranteed visible.

- Immediately after the acquire-load confirms AVAILABLE, all
  referenced fields (texture, width, height, alpha, flags) are
  copied into stack locals.  Only these snapshots are used for
  the remainder of the draw call — the live struct is not touched
  again.
- gfx_thumbnail_get_draw_dimensions() receives a stack-local
  gfx_thumbnail_t built from the snapshots, so it also operates
  on consistent data without signature changes.

- draw.texture uses the snapshotted handle, not the live field.

Platform support:

  GCC/Clang: __atomic_store_n / __atomic_load_n with
             __ATOMIC_RELEASE / __ATOMIC_ACQUIRE.
             Compiles to STLR/LDAR on ARM64 (single-cycle),
             plain MOV on x86 (free — TSO provides the
             guarantee), LWSYNC/LD on PowerPC.

  MSVC:      _InterlockedExchange / _InterlockedOr.
             Only targets x86/x64 in RetroArch's matrix.

  Fallback:  Plain volatile read/write.  Sufficient on
             single-core platforms and x86.  Platforms with
             weak ordering that lack these builtins do not
             run threaded video.

No external call site changes.  No signature changes.
No behavioral or visual changes.  Stack cost is ~32 bytes
(5 snapshot locals + 1 temp gfx_thumbnail_t in a nested scope).
This commit is contained in:
U-DESKTOP-SPFP6AQ\twistedtechre committed 2026-04-16 04:41:11 +02:00
1 parent 0abe6dc47d
commit 50931c0a84
1 file changed
+81 -12
+81 -12
View File
@@ -43,6 +43,40 @@
#define DEFAULT_GFX_THUMBNAIL_STREAM_DELAY 16.66667f * 3
#define DEFAULT_GFX_THUMBNAIL_FADE_DURATION 166.66667f
/* Helpers for cross-thread thumbnail status synchronisation.
*
* The main thread writes texture/width/height then publishes
* status = AVAILABLE. The video thread reads status; if it
* sees AVAILABLE, the data fields must be visible.
*
* On GCC/Clang (covers ARM64, x86, PowerPC, MIPS):
* release-store / acquire-load give the required ordering.
* On MSVC (x86/x64 only in RetroArch's target matrix):
* x86 total-store-order makes plain volatile sufficient,
* but _InterlockedOr gives an explicit acquire load.
* Fallback:
* Plain volatile read/write — sufficient on single-core
* and x86. Platforms with weak ordering that lack GCC
* builtins do not run threaded video in practice.
*/
#if defined(__GNUC__)
#define GFX_THUMB_STATUS_STORE(ptr, val) \
__atomic_store_n((int*)(ptr), (int)(val), __ATOMIC_RELEASE)
#define GFX_THUMB_STATUS_LOAD(ptr) \
((enum gfx_thumbnail_status)__atomic_load_n((int*)(ptr), __ATOMIC_ACQUIRE))
#elif defined(_MSC_VER)
#include <intrin.h>
#define GFX_THUMB_STATUS_STORE(ptr, val) \
_InterlockedExchange((volatile long*)(ptr), (long)(val))
#define GFX_THUMB_STATUS_LOAD(ptr) \
((enum gfx_thumbnail_status)_InterlockedOr((volatile long*)(ptr), 0))
#else
#define GFX_THUMB_STATUS_STORE(ptr, val) \
do { (*(volatile int*)(ptr)) = (int)(val); } while(0)
#define GFX_THUMB_STATUS_LOAD(ptr) \
((enum gfx_thumbnail_status)(*(volatile int*)(ptr)))
#endif
/* Utility structure, sent as userdata when pushing
* an image load */
typedef struct
@@ -174,8 +208,11 @@ static void gfx_thumbnail_handle_upload(
gfx_thumbnail_reset(thumbnail_tag->thumbnail);
/* Set thumbnail 'missing' status by default
* (saves a number of checks later) */
thumbnail_tag->thumbnail->status = GFX_THUMBNAIL_STATUS_MISSING;
* (saves a number of checks later)
* > Release-store ensures prior texture reset is
* visible before status change */
GFX_THUMB_STATUS_STORE(&thumbnail_tag->thumbnail->status,
GFX_THUMBNAIL_STATUS_MISSING);
/* If we reach this stage, thumbnail 'fade in'
* animations should be applied (based on current
@@ -196,8 +233,12 @@ static void gfx_thumbnail_handle_upload(
thumbnail_tag->thumbnail->width = img->width;
thumbnail_tag->thumbnail->height = img->height;
/* Update thumbnail status */
thumbnail_tag->thumbnail->status = GFX_THUMBNAIL_STATUS_AVAILABLE;
/* Update thumbnail status
* > Release-store ensures texture/width/height writes
* are visible to the video thread before it sees
* AVAILABLE via acquire-load in gfx_thumbnail_draw() */
GFX_THUMB_STATUS_STORE(&thumbnail_tag->thumbnail->status,
GFX_THUMBNAIL_STATUS_AVAILABLE);
end:
/* Clean up */
@@ -945,8 +986,12 @@ void gfx_thumbnail_draw(
)
return;
/* Only draw thumbnail if it is available... */
if (thumbnail->status == GFX_THUMBNAIL_STATUS_AVAILABLE)
/* Only draw thumbnail if it is available...
* > Acquire-load pairs with release-store in
* gfx_thumbnail_handle_upload() to ensure
* texture/width/height are visible when we
* read AVAILABLE */
if (GFX_THUMB_STATUS_LOAD(&thumbnail->status) == GFX_THUMBNAIL_STATUS_AVAILABLE)
{
gfx_display_ctx_draw_t draw;
struct video_coords coords;
@@ -955,7 +1000,18 @@ void gfx_thumbnail_draw(
float draw_height;
float draw_x;
float draw_y;
float thumbnail_alpha = thumbnail->alpha * alpha;
/* Snapshot fields — read once, use local copies for the
* remainder of this draw call. The main thread may update
* the live struct concurrently (upload callback, reset, or
* animation tick on alpha), but these locals are stable. */
uintptr_t thumb_texture = thumbnail->texture;
unsigned thumb_width = thumbnail->width;
unsigned thumb_height = thumbnail->height;
float thumb_alpha = thumbnail->alpha;
uint8_t thumb_flags = thumbnail->flags;
float thumbnail_alpha = thumb_alpha * alpha;
float thumbnail_color[16] = {
1.0f, 1.0f, 1.0f, 1.0f,
1.0f, 1.0f, 1.0f, 1.0f,
@@ -969,10 +1025,23 @@ void gfx_thumbnail_draw(
if (thumbnail_alpha < 1.0f)
gfx_display_set_alpha(thumbnail_color, thumbnail_alpha);
/* Get thumbnail dimensions */
gfx_thumbnail_get_draw_dimensions(
thumbnail, width, height, scale_factor,
&draw_width, &draw_height);
/* Get thumbnail dimensions using snapshot values
* > Build a temporary on the stack so
* gfx_thumbnail_get_draw_dimensions reads
* consistent data without touching the live struct */
{
gfx_thumbnail_t thumb_snapshot;
thumb_snapshot.texture = thumb_texture;
thumb_snapshot.width = thumb_width;
thumb_snapshot.height = thumb_height;
thumb_snapshot.alpha = thumb_alpha;
thumb_snapshot.flags = thumb_flags;
thumb_snapshot.status = GFX_THUMBNAIL_STATUS_AVAILABLE;
thumb_snapshot.delay_timer = 0.0f;
gfx_thumbnail_get_draw_dimensions(
&thumb_snapshot, width, height, scale_factor,
&draw_width, &draw_height);
}
if (dispctx->blend_begin)
dispctx->blend_begin(userdata);
@@ -1006,7 +1075,7 @@ void gfx_thumbnail_draw(
draw.rotation = 0.0f;
draw.coords = &coords;
draw.matrix_data = &mymat;
draw.texture = thumbnail->texture;
draw.texture = thumb_texture;
draw.prim_type = GFX_DISPLAY_PRIM_TRIANGLESTRIP;
draw.pipeline_id = 0;