video: the window title crosses threads in an atomic mailbox

The title handoff was the last thing the video thread took
display_lock for on its frame path: the main thread assembles the
title once per fps interval and every driver's frame() offers to
apply it, with the 512-byte string uncopyable in one atomic step.
The lock's job was only to prevent a torn copy - so the copy stops
being shared at all. Where the atomics have pointer ops the title
crosses as an immutable heap copy through a one-slot mailbox:
publish is exchange-in, freeing whatever title was never taken, and
take is exchange-out. A copy either is not published or is
complete, because nothing writes it after publication; both sides
are wait-free; a slow reader costs a superseded title, never a
leak; and the read-side fast path stays one acquire load. The win
is not the lock's cycles - it was uncontended - but the lock-order
edge it removes: display_lock also guards the disp flags and the
viewport rectangle, and the context drivers already document
context_lock + display_lock ordering on the video thread, so the
title no longer participates in that graph at all.

On the one backend without pointer atomics, and in builds without
threads, the old protocol stands unchanged behind the same macros,
per retro_atomic.h's documented gate - the mpsc deferral's
precedent. samples/gfx/window_title_mailbox reproduces the protocol
and holds three claims across 20000 paced publishes: no torn copy
(embedded checksums), supersession only forward with the final
title always taken, and every copy freed exactly once under ASan.
Clean under Clang ThreadSanitizer; a write-after-publish sabotage
lights up all three checks. All five atomic backends compile the
new video_driver.c, VOLATILE on the lock path; C89 holds; the
74-lane matrix and all four hot-function budgets pass.
This commit is contained in:
LibretroAdmin authored and libretroadmin committed 2026-09-15 17:24:56 +02:00
1 parent dc8d39a347
commit 0cb8a462cf
4 files changed
+277 -11

No files matched your search

+70 -8
View File
@@ -2630,6 +2630,16 @@ void video_driver_lock_new(void)
video_st->display_lock = slock_new();
if (!video_st->context_lock)
video_st->context_lock = slock_new();
#ifdef VIDEO_TITLE_MAILBOX
/* Runs before the video thread exists; drop anything stale from a
* previous driver's life and start empty. */
{
char *stale = (char*)retro_atomic_exchange_ptr(
&video_st->window_title_pending, NULL);
if (stale)
free(stale);
}
#endif
#endif
}
@@ -4331,11 +4341,21 @@ bool video_driver_has_focus(void)
return VIDEO_HAS_FOCUS(video_st);
}
/* window_title is written on the main thread and read on the video
* thread, so both sides use display_lock around it; window_title_update
* says there is a new one. No-ops in a build without threads, where
* there is no second reader. */
#ifdef HAVE_THREADS
/* The window title crosses from the main thread, which builds it, to
* the video thread, which applies it. Where the atomics have pointer
* ops it crosses as an immutable heap copy through a one-slot atomic
* mailbox (see video_driver.h): exchange in, exchange out, no lock
* and no possible torn copy - a copy either is not published yet or
* is complete, because nothing writes it after publication. The
* exchange-in frees whatever title was never taken, so a slow reader
* costs a superseded title, never a leak. On the one backend without
* pointer ops (and in a build without threads, where there is no
* second thread) the old display_lock protocol stands below,
* unchanged. */
#if defined(HAVE_THREADS) && defined(RETRO_ATOMIC_HAS_PTR)
#define VIDEO_TITLE_MAILBOX 1
#endif
#if defined(HAVE_THREADS) && !defined(VIDEO_TITLE_MAILBOX)
#define VIDEO_TITLE_LOCK(st) do { if ((st)->display_lock) slock_lock((st)->display_lock); } while (0)
#define VIDEO_TITLE_UNLOCK(st) do { if ((st)->display_lock) slock_unlock((st)->display_lock); } while (0)
#else
@@ -4343,6 +4363,23 @@ bool video_driver_has_focus(void)
#define VIDEO_TITLE_UNLOCK(st) ((void)0)
#endif
#ifdef VIDEO_TITLE_MAILBOX
/* Publish window_title (main-thread scratch here) as the pending
* title. Failure to allocate skips one title update - the next fps
* interval brings another. */
static void video_title_publish(video_driver_state_t *video_st)
{
char *copy = strdup(video_st->window_title);
if (copy)
{
char *stale = (char*)retro_atomic_exchange_ptr(
&video_st->window_title_pending, copy);
if (stale)
free(stale);
}
}
#endif
/* Hands the pending window title to the caller and clears the pending
* bit, so the next caller gets nothing until a new title is built.
*
@@ -4362,6 +4399,20 @@ size_t video_driver_get_window_title(char *s, size_t len)
if (!s)
return 0;
#ifdef VIDEO_TITLE_MAILBOX
/* Nearly every frame has no new title */
if (!retro_atomic_load_acquire_ptr(&video_st->window_title_pending))
return 0;
{
char *taken = (char*)retro_atomic_exchange_ptr(
&video_st->window_title_pending, NULL);
if (taken)
{
n = strlcpy(s, taken, len);
free(taken);
}
}
#else
/* Nearly every frame has no new title */
if (!retro_atomic_load_acquire_int(&video_st->window_title_update))
return 0;
@@ -4373,6 +4424,7 @@ size_t video_driver_get_window_title(char *s, size_t len)
retro_atomic_store_release_int(&video_st->window_title_update, 0);
}
VIDEO_TITLE_UNLOCK(video_st);
#endif
if (!n)
return 0;
@@ -6077,9 +6129,11 @@ void video_driver_frame(const void *data, unsigned width,
last_fps = TIME_TO_FPS(curr_time, new_time,
fps_update_interval);
/* Built under display_lock, and the pending bit set inside
* it, so the video thread's video_driver_get_window_title()
* cannot copy a title halfway through being assembled. */
/* Under the mailbox, window_title is this thread's scratch
* and publishing is the exchange in video_title_publish();
* under the lock protocol the assembly runs inside
* display_lock so the video thread cannot copy a title
* halfway through being assembled. */
VIDEO_TITLE_LOCK(video_st);
__len = strlcpy(video_st->window_title, video_st->title_buf,
sizeof(video_st->window_title));
@@ -6097,7 +6151,11 @@ void video_driver_frame(const void *data, unsigned width,
sizeof(video_st->window_title) - __len);
}
#ifdef VIDEO_TITLE_MAILBOX
video_title_publish(video_st);
#else
retro_atomic_store_release_int(&video_st->window_title_update, 1);
#endif
VIDEO_TITLE_UNLOCK(video_st);
curr_time = new_time;
@@ -6112,7 +6170,11 @@ void video_driver_frame(const void *data, unsigned width,
video_st->window_title,
video_st->title_buf,
sizeof(video_st->window_title));
#ifdef VIDEO_TITLE_MAILBOX
video_title_publish(video_st);
#else
retro_atomic_store_release_int(&video_st->window_title_update, 1);
#endif
VIDEO_TITLE_UNLOCK(video_st);
status_text[0] = '\0';
+13 -3
View File
@@ -1251,10 +1251,20 @@ typedef struct
char cli_shader_path[PATH_MAX_LENGTH];
char window_title[512];
char window_title_prev[512];
/* A new window_title waits for the thread that draws: raised with
* the title, under display_lock, and taken with it by
* video_driver_get_window_title(), which reads this first so a frame
/* A new window title waits for the thread that draws. Where the
* atomics have pointer ops (RETRO_ATOMIC_HAS_PTR) it waits as an
* immutable heap copy in window_title_pending - a one-slot atomic
* mailbox: the main thread exchanges a fresh copy in (freeing any
* title never taken), video_driver_get_window_title() exchanges it
* out. No tearing is possible and neither side takes a lock;
* window_title is then main-thread scratch. On the one backend
* without pointer ops the old protocol stands: window_title is
* shared, rewritten and copied under display_lock, with
* window_title_update as the pending bit read first so a frame
* with no new title takes no lock. */
#if defined(HAVE_THREADS) && defined(RETRO_ATOMIC_HAS_PTR)
retro_atomic_ptr_t window_title_pending;
#endif
retro_atomic_int_t window_title_update;
char gpu_api_version_string[128];
char title_buf[64];
+35
View File
@@ -0,0 +1,35 @@
TARGET := window_title_mailbox_test
LIBRETRO_COMM_DIR := ../../../libretro-common
# Self-contained: the hazard protocol under test is reproduced in the
# harness (see the file header for why), so the only tree dependency
# is rthreads.c for portable threads and locks. retro_atomic.h and
# retro_miscellaneous.h are header-only.
SOURCES := \
window_title_mailbox_test.c \
$(LIBRETRO_COMM_DIR)/rthreads/rthreads.c
OBJS := $(SOURCES:.c=.o)
CFLAGS += -Wall -std=gnu99 -g -O1 \
-DHAVE_THREADS -I$(LIBRETRO_COMM_DIR)/include
LDFLAGS += -lpthread -lrt
ifneq ($(SANITIZER),)
CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS)
LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS)
endif
all: $(TARGET)
$(TARGET): $(OBJS)
$(CC) -o $@ $(OBJS) $(LDFLAGS)
check: all
./$(TARGET)
clean:
rm -f $(OBJS) $(TARGET)
.PHONY: all check clean
@@ -0,0 +1,159 @@
/* window_title_mailbox_test.c -- the one-slot atomic title mailbox of
* gfx/video_driver.c, reproduced and beaten on.
*
* The protocol: the main thread hands the window title to the video
* thread as an immutable heap copy through one retro_atomic_ptr_t
* slot. Publish is exchange-in (freeing whatever was never taken),
* take is exchange-out. The claims a regression would break:
*
* 1. No torn copy: every title the consumer takes is internally
* consistent (its embedded checksum holds), because a published
* copy is never written again.
* 2. No leak and no double free: every copy is freed exactly once,
* by whichever side superseded or took it (ASan holds this).
* 3. Supersession only forward: the sequence numbers the consumer
* observes strictly increase, and the last published title is
* taken once the producer stops.
*
* The harness reproduces the protocol rather than including
* video_driver.c (which drags the whole frontend in); the shape is a
* dozen lines and drift is caught by eye at the publish/take sites.
*
* Paced with short sleeps so a single-processor machine interleaves
* the two threads; the protocol has no spin anywhere. */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <retro_atomic.h>
#include <rthreads/rthreads.h>
#ifndef RETRO_ATOMIC_HAS_PTR
int main(void)
{
printf("window_title_mailbox: no pointer atomics on this backend; "
"the lock protocol stands and this test does not apply\n");
return 0;
}
#else
#if defined(_WIN32)
#include <windows.h>
static void sleep_us(unsigned us) { Sleep(us / 1000u + 1u); }
#else
#include <unistd.h>
static void sleep_us(unsigned us) { usleep(us); }
#endif
#define N_TITLES 20000
static retro_atomic_ptr_t g_pending;
static retro_atomic_int_t g_stop;
static unsigned title_checksum(const char *body)
{
unsigned h = 2166136261u;
for (; *body; body++)
h = (h ^ (unsigned char)*body) * 16777619u;
return h;
}
static char *title_make(int seq)
{
char body[64], *s = (char*)malloc(96);
if (!s)
abort();
snprintf(body, sizeof(body), "core 1.0 || seq %d fps 60.0", seq);
snprintf(s, 96, "%s#%08x", body, title_checksum(body));
return s;
}
/* Returns the sequence number, or -1 on a torn/corrupt title. */
static int title_check(const char *s)
{
char body[64];
unsigned want;
int seq;
const char *hash = strrchr(s, '#');
if (!hash || (size_t)(hash - s) >= sizeof(body))
return -1;
memcpy(body, s, (size_t)(hash - s));
body[hash - s] = '\0';
if (sscanf(hash + 1, "%x", &want) != 1)
return -1;
if (title_checksum(body) != want)
return -1;
if (sscanf(body, "core 1.0 || seq %d", &seq) != 1)
return -1;
return seq;
}
static void producer(void *arg)
{
int i;
(void)arg;
for (i = 0; i < N_TITLES; i++)
{
char *copy = title_make(i);
char *stale = (char*)retro_atomic_exchange_ptr(&g_pending, copy);
if (stale)
free(stale);
if ((i & 63) == 0)
sleep_us(100);
}
retro_atomic_store_release_int(&g_stop, 1);
}
int main(void)
{
sthread_t *p;
int last_seq = -1, taken = 0, torn = 0, order = 0;
retro_atomic_ptr_init(&g_pending, NULL);
retro_atomic_store_relaxed_int(&g_stop, 0);
p = sthread_create(producer, NULL);
for (;;)
{
int stopped = retro_atomic_load_acquire_int(&g_stop);
if (retro_atomic_load_acquire_ptr(&g_pending))
{
char *t = (char*)retro_atomic_exchange_ptr(&g_pending, NULL);
if (t)
{
int seq = title_check(t);
if (seq < 0)
torn++;
else if (seq <= last_seq)
order++;
else
last_seq = seq;
free(t);
taken++;
}
}
else if (stopped)
break;
sleep_us(50);
}
sthread_join(p);
printf("window_title_mailbox: %d published, %d taken, last seq %d\n",
N_TITLES, taken, last_seq);
if (torn)
printf(" FAIL: %d torn/corrupt title(s) taken\n", torn);
if (order)
printf(" FAIL: %d title(s) out of order\n", order);
if (last_seq != N_TITLES - 1)
printf(" FAIL: final title lost (last seq %d, want %d)\n",
last_seq, N_TITLES - 1);
if (torn || order || last_seq != N_TITLES - 1)
{
printf("window_title_mailbox: FAILED\n");
return 1;
}
printf("window_title_mailbox: ok\n");
return 0;
}
#endif /* RETRO_ATOMIC_HAS_PTR */