audio: the Device list is the configured driver's, and is freed by the driver that built it

Picking a driver in the menu writes the setting and rebuilds the page;
it does not reinitialise audio, so the running driver stays what it
was until a restart. The Device list is rebuilt each time its screen
opens, and it was asked of the running driver - so after choosing asio
it still listed wasapi's devices, under asio's name, until the next
start. The Device setting is applied to the configured driver, and its
list now comes from the configured driver: from the running instance
when that is the one configured, from the configured driver with no
context otherwise, which enumeration allows.

A list can now be built by a driver other than the one running when it
is released, so the builder is recorded with the list and its
device_list_free is the one called. Every such free in the tree is a
plain string_list_free today; the record keeps a mismatch from
mattering when one is not.

samples/audio/device_list_driver runs the shipping selection with alsa
running on the null PCM and null configured, and asserts the list is
not the running driver's; the previous selection returns alsa's
devices there. On the audio samples workflow. Clean under
ASan/UBSan/LSan and TSan; C89 checked, and the header's Windows
consumers pass tools/mingw_syntax_check.sh.
This commit is contained in:
LibretroAdmin authored and libretroadmin committed 2026-09-02 20:19:08 +02:00
1 parent ff8f578487
commit cde1e88203
6 files changed
+553 -13

No files matched your search

+21
View File
@@ -172,3 +172,24 @@ jobs:
make clean all SANITIZER=thread
TSAN_OPTIONS=halt_on_error=1 timeout 400 ./pipeline_stall_test
echo "[pass] pipeline_stall_test (TSan)"
- name: Build and run device_list_driver_test (ASan + UBSan)
shell: bash
working-directory: samples/audio/device_list_driver
run: |
set -eu
# Includes audio/audio_driver.c with the alsa and null drivers in
# the table. Picking a driver in the menu writes the setting but
# does not reinitialise audio, and the Device list - rebuilt on
# each open - was asked of the running driver, so it showed the
# old driver's devices under the new driver's name until a
# restart. The list now comes from the configured driver, and
# is freed by the driver that built it. Cases: configured and
# running agree; configured differs from running; back again;
# nothing running; and release by the builder, once.
make clean all SANITIZER=address,undefined
test -x device_list_driver_test
ASAN_OPTIONS=detect_leaks=1:detect_stack_use_after_return=1 \
UBSAN_OPTIONS=print_stacktrace=1 timeout 120 \
./device_list_driver_test
echo "[pass] device_list_driver_test (ASan)"
+32 -13
View File
@@ -371,13 +371,20 @@ bool audio_driver_is_ai_service_speech_running(void)
}
#endif
/* The driver whose device_list_new to call: the wrapped driver under
* the threaded pipeline, or the configured driver when nothing is
* initialised at all. NULL context is legal for enumeration. */
/* The driver whose device_list_new to call. The devices listed are the
* ones the Device setting will be applied to, and that is the
* configured driver: when it is the one running, ask it, through the
* wrapper if the threaded pipeline is on; when the user has picked a
* different driver in the menu and nothing has reinitialised yet, ask
* the configured driver with no context, which enumeration allows.
* Asking the running driver in that case listed the old driver's
* devices under the new driver's name until the next restart. */
static const audio_driver_t *audio_driver_enumeration_driver(
audio_driver_state_t *audio_st, void **ctx)
{
settings_t *settings = config_get_ptr();
const audio_driver_t *audio = audio_st->current_audio;
const char *running_ident = audio ? audio->ident : NULL;
*ctx = audio_st->context_audio_data;
#ifdef HAVE_THREADS
if (audio && string_is_equal(audio->ident, "audio-thread"))
@@ -385,15 +392,22 @@ static const audio_driver_t *audio_driver_enumeration_driver(
const audio_driver_t *inner =
audio_thread_wrapped_driver(audio_st->context_audio_data);
if (inner)
return audio; /* the wrapper forwards, context intact */
/* Wrapper selected but never started: no inner driver to ask.
* Fall through to the configured one with no context. */
audio = NULL;
running_ident = inner->ident; /* the wrapper forwards, context intact */
else
{
/* Wrapper selected but never started: no inner driver to ask.
* Fall through to the configured one with no context. */
audio = NULL;
running_ident = NULL;
}
}
#endif
if ( audio
&& running_ident
&& !string_is_equal(running_ident, settings->arrays.audio_driver))
audio = NULL;
if (!audio)
{
settings_t *settings = config_get_ptr();
int i = (int)driver_find_index("audio_driver",
settings->arrays.audio_driver);
if (i >= 0)
@@ -406,16 +420,18 @@ static const audio_driver_t *audio_driver_enumeration_driver(
static bool audio_driver_free_devices_list(void)
{
audio_driver_state_t *audio_st = &audio_driver_st;
void *ctx = NULL;
const audio_driver_t *audio;
const audio_driver_t *audio = audio_st->devices_list_driver;
if (!audio_st->devices_list)
return false;
audio = audio_driver_enumeration_driver(audio_st, &ctx);
/* Released by the driver that built it. The context it was built
* with may be gone by now; every driver's free takes the list alone
* and tolerates a NULL context. */
if (audio && audio->device_list_free)
audio->device_list_free(ctx, audio_st->devices_list);
audio->device_list_free(NULL, audio_st->devices_list);
else
string_list_free(audio_st->devices_list);
audio_st->devices_list = NULL;
audio_st->devices_list = NULL;
audio_st->devices_list_driver = NULL;
return true;
}
@@ -427,8 +443,11 @@ void audio_driver_refresh_devices_list(void)
audio_driver_free_devices_list();
audio = audio_driver_enumeration_driver(audio_st, &ctx);
if (audio && audio->device_list_new)
{
audio_st->devices_list = (struct string_list*)
audio->device_list_new(ctx);
audio_st->devices_list_driver = audio;
}
}
#ifdef DEBUG
+4
View File
@@ -272,6 +272,10 @@ typedef struct
uint64_t free_samples_count;
struct string_list *devices_list;
/* The driver whose device_list_new built devices_list, so that its
* device_list_free is the one that releases it - not whichever
* driver is configured or running when the list is next rebuilt. */
const audio_driver_t *devices_list_driver;
/**
* A scratch buffer for audio output to be processed,
+76
View File
@@ -0,0 +1,76 @@
compiler := gcc
TARGET := device_list_driver_test
CORE_DIR = ../../..
LIBRETRO_COMM_DIR = $(CORE_DIR)/libretro-common
INCDIRS := -I$(LIBRETRO_COMM_DIR)/include -I$(CORE_DIR) \
-I$(CORE_DIR)/deps -I$(CORE_DIR)/deps/stb
CC := $(compiler)
flags += -std=gnu99
SOURCES_C := \
device_list_driver_test.c \
stubs_retroarch.c \
$(CORE_DIR)/audio/drivers/alsa.c \
$(CORE_DIR)/audio/common/alsa.c \
$(LIBRETRO_COMM_DIR)/lists/string_list.c \
$(LIBRETRO_COMM_DIR)/lists/dir_list.c \
$(LIBRETRO_COMM_DIR)/file/file_path.c \
$(LIBRETRO_COMM_DIR)/file/retro_dirent.c \
$(LIBRETRO_COMM_DIR)/streams/file_stream.c \
$(LIBRETRO_COMM_DIR)/vfs/vfs_implementation.c \
$(LIBRETRO_COMM_DIR)/rthreads/rthreads.c \
$(LIBRETRO_COMM_DIR)/queues/retro_spsc.c \
$(LIBRETRO_COMM_DIR)/queues/fifo_queue.c \
$(LIBRETRO_COMM_DIR)/audio/conversion/float_to_s16.c \
$(LIBRETRO_COMM_DIR)/audio/conversion/s16_to_float.c \
$(LIBRETRO_COMM_DIR)/audio/resampler/audio_resampler.c \
$(LIBRETRO_COMM_DIR)/audio/resampler/drivers/sinc_resampler_int16.c \
$(LIBRETRO_COMM_DIR)/audio/resampler/drivers/nearest_resampler_int16.c \
$(LIBRETRO_COMM_DIR)/features/features_cpu.c \
$(LIBRETRO_COMM_DIR)/memmap/memalign.c \
$(LIBRETRO_COMM_DIR)/string/stdstring.c \
$(LIBRETRO_COMM_DIR)/string/rstrtod.c \
$(LIBRETRO_COMM_DIR)/time/rtime.c \
$(LIBRETRO_COMM_DIR)/file/file_path_io.c \
$(LIBRETRO_COMM_DIR)/file/config_file.c \
$(LIBRETRO_COMM_DIR)/file/config_file_userdata.c \
$(LIBRETRO_COMM_DIR)/audio/resampler/drivers/sinc_resampler.c \
$(LIBRETRO_COMM_DIR)/audio/resampler/drivers/nearest_resampler.c \
$(LIBRETRO_COMM_DIR)/compat/compat_strl.c \
$(LIBRETRO_COMM_DIR)/compat/compat_strldup.c \
$(LIBRETRO_COMM_DIR)/compat/fopen_utf8.c \
$(LIBRETRO_COMM_DIR)/encodings/encoding_utf.c
CFLAGS += -Wall $(INCDIRS)
CFLAGS += -DRARCH_INTERNAL -DHAVE_THREADS -DHAVE_AUDIOMIXER -DHAVE_MENU \
-DHAVE_CONFIGFILE -DHAVE_ALSA -D_GNU_SOURCE
LDFLAGS += -lm -lpthread -lasound
ifneq ($(SAN),)
SANITIZER := $(SAN)
endif
ifneq ($(SANITIZER),)
CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer -g $(CFLAGS)
LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS)
endif
OBJS := $(SOURCES_C:.c=.o)
all: $(TARGET)
%.o: %.c
$(CC) $(INCDIRS) $(CFLAGS) $(flags) -c -o$@ $<
$(TARGET): $(OBJS)
$(CC) -o$@ $(OBJS) $(LDFLAGS)
check: $(TARGET)
./$(TARGET)
clean:
rm -f $(OBJS)
rm -f $(TARGET)
.PHONY: clean all check
@@ -0,0 +1,121 @@
/* The audio Device list after the driver setting changes.
*
* Picking a driver in the menu writes the setting and rebuilds the
* page; it does not reinitialise audio. The running driver stays what
* it was until a restart, and the device list - rebuilt each time the
* Device screen opens - was asked of the running driver, so it showed
* the old driver's devices under the new driver's name. The Device
* setting is applied to the configured driver, so its list has to
* come from the configured driver: from the running instance when
* that is the one configured, from the configured driver with no
* context otherwise.
*
* With the list now able to come from a driver other than the one
* running, the driver that built it is recorded with it and is the
* one that frees it. Every device_list_free in the tree is a plain
* string_list_free today, so a mismatch would cost nothing today; the
* record is so that stays true when one is not.
*
* Includes audio/audio_driver.c so the shipping selection runs. Two
* real drivers from the table: alsa, enumerating on this box through
* libasound's hints, and null, which enumerates nothing. Under ASan so
* a list freed by the wrong driver, or twice, or not at all, shows. */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <boolean.h>
#include "../../../audio/audio_driver.c"
static unsigned failures = 0;
#define CHECK(cond, ...) \
do { \
if (!(cond)) \
{ \
printf("FAIL %s:%d: ", __FILE__, __LINE__); \
printf(__VA_ARGS__); \
printf("\n"); \
failures++; \
} \
} while (0)
static void configure(const char *ident)
{
settings_t *settings = config_get_ptr();
strlcpy(settings->arrays.audio_driver, ident,
sizeof(settings->arrays.audio_driver));
}
int main(void)
{
audio_driver_state_t *st = &audio_driver_st;
void *list = NULL;
unsigned new_rate = 0;
void *alsa_ctx;
/* alsa running on the null PCM, as after a normal init. */
alsa_ctx = audio_alsa.init("null", 48000, 64, 1024, &new_rate);
if (!alsa_ctx)
{
printf("SKIP: null PCM unavailable\n");
return 0;
}
st->current_audio = &audio_alsa;
st->context_audio_data = alsa_ctx;
/* 1. Configured driver is the running one: enumerate through it. */
configure("alsa");
audio_driver_refresh_devices_list();
CHECK(st->devices_list_driver == &audio_alsa,
"alsa configured and running: list not built by alsa");
CHECK(audio_driver_get_devices_list(&list) && list,
"alsa configured and running: no list");
/* 2. The user picks null in the menu; alsa is still running. The
* list must now be null's - which enumerates nothing - not
* alsa's devices under null's name. The alsa-built list is freed
* by alsa. */
configure("null");
audio_driver_refresh_devices_list();
CHECK(st->devices_list_driver != &audio_alsa,
"null configured, alsa running: list still attributed to alsa");
list = NULL;
CHECK(!audio_driver_get_devices_list(&list) || !list,
"null configured, alsa running: a device list came from the running driver");
/* 3. Back to alsa: its list again, from the running instance. */
configure("alsa");
audio_driver_refresh_devices_list();
CHECK(st->devices_list_driver == &audio_alsa,
"alsa reconfigured: list not built by alsa");
list = NULL;
CHECK(audio_driver_get_devices_list(&list) && list,
"alsa reconfigured: no list");
/* 4. Nothing running at all, alsa configured: enumerate with no
* context, which enumeration allows. */
audio_alsa.free(alsa_ctx);
st->current_audio = NULL;
st->context_audio_data = NULL;
audio_driver_refresh_devices_list();
CHECK(st->devices_list_driver == &audio_alsa,
"alsa configured, nothing running: list not built by alsa");
/* 5. Release: by the builder, once. */
audio_driver_free_devices_list();
CHECK(!st->devices_list && !st->devices_list_driver,
"free left the list or its builder behind");
CHECK(!audio_driver_free_devices_list(),
"a second free reported work to do");
if (failures)
{
printf("%u failure(s)\n", failures);
return 1;
}
printf("device list: follows the configured driver, freed by its builder\n");
return 0;
}
@@ -0,0 +1,299 @@
/* Frontend, mixer and resampler symbols audio/audio_driver.c
* references. The subject of this harness is the driver's own locking
* around mixer_streams[], so the mixer stubs are just enough to make
* load/play/stop/destroy produce distinguishable non-NULL handles and
* voices - the state machine that add_stream and stop_stream drive
* lives in audio_driver.c and is exercised for real.
*
* Signatures are copied from the tree's headers rather than guessed.
* Anything the driver should not reach on these paths aborts instead
* of quietly returning. */
#include <stdio.h>
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
#include <boolean.h>
#include <audio/audio_mixer.h>
#include "../../../configuration.h"
#include "../../../record/record_driver.h"
#include "../../../runloop.h"
#include "../../../menu/menu_driver.h"
#include "../../../gfx/video_driver.h"
#include <string/stdstring.h>
#include "../../../audio/audio_driver.h"
#include "../../../command.h"
void RARCH_LOG(const char *fmt, ...) { (void)fmt; }
void RARCH_WARN(const char *fmt, ...) { (void)fmt; }
void RARCH_DBG(const char *fmt, ...) { (void)fmt; }
void RARCH_LOG_OUTPUT(const char *fmt, ...) { (void)fmt; }
void RARCH_ERR(const char *fmt, ...)
{
va_list ap;
va_start(ap, fmt);
printf("ERR: ");
vprintf(fmt, ap);
va_end(ap);
}
bool verbosity_is_enabled(void) { return false; }
settings_t *config_get_ptr(void)
{
static settings_t settings;
return &settings;
}
static runloop_state_t runloop_st;
runloop_state_t *runloop_state_get_ptr(void) { return &runloop_st; }
uint32_t runloop_get_flags(void) { return 0; }
static recording_state_t recording_st;
recording_state_t *recording_state_get_ptr(void) { return &recording_st; }
static video_driver_state_t video_st;
video_driver_state_t *video_state_get_ptr(void) { return &video_st; }
static struct menu_state menu_st;
struct menu_state *menu_state_get_ptr(void) { return &menu_st; }
/* --- mixer ----------------------------------------------------------- */
/* Distinct non-NULL cookies: the driver only ever stores and compares
* these, and freeing them here would hide a use-after-free in the
* driver rather than expose one, so each allocation is real. */
/* The real audio_mixer_destroy() frees the buffer the sound was loaded
* from for the streaming types (it owns it once loaded, unless it was
* borrowed via data_owner). The stub mirrors that ownership so leak
* checking over these paths means something. */
struct audio_mixer_sound { void *owned_data; };
struct audio_mixer_voice { int tag; };
static audio_mixer_sound_t *new_sound_owning(void *data)
{
audio_mixer_sound_t *s = (audio_mixer_sound_t*)calloc(1, sizeof(*s));
if (s)
s->owned_data = data;
return s;
}
static audio_mixer_sound_t *new_sound(void)
{
return new_sound_owning(NULL);
}
static audio_mixer_voice_t *new_voice(void)
{
audio_mixer_voice_t *v = (audio_mixer_voice_t*)calloc(1, sizeof(*v));
return v;
}
void audio_mixer_init(unsigned rate) { (void)rate; }
void audio_mixer_done(void) { }
audio_mixer_sound_t *audio_mixer_load_wav(void *buffer, size_t size,
const char *resampler_ident, enum resampler_quality quality,
bool want_s16)
{
(void)buffer; (void)size; (void)resampler_ident; (void)quality;
(void)want_s16;
return new_sound();
}
audio_mixer_sound_t *audio_mixer_load_wav_stream(void *buffer,
size_t size)
{
(void)size;
return new_sound_owning(buffer);
}
audio_mixer_sound_t *audio_mixer_load_ogg(void *buffer, size_t size)
{
(void)size;
return new_sound_owning(buffer);
}
audio_mixer_sound_t *audio_mixer_load_mod(void *buffer, size_t size)
{
(void)size;
return new_sound_owning(buffer);
}
void audio_mixer_destroy(audio_mixer_sound_t *sound)
{
if (!sound)
return;
if (sound->owned_data)
free(sound->owned_data);
free(sound);
}
audio_mixer_voice_t *audio_mixer_play(audio_mixer_sound_t *sound,
bool repeat, float volume, const char *resampler_ident,
enum resampler_quality quality, audio_mixer_stop_cb_t stop_cb)
{
(void)sound; (void)repeat; (void)volume; (void)resampler_ident;
(void)quality; (void)stop_cb;
return new_voice();
}
audio_mixer_voice_t *audio_mixer_play_s16(audio_mixer_sound_t *sound,
bool repeat, int32_t gain, enum resampler_quality quality,
audio_mixer_stop_cb_t stop_cb)
{
(void)sound; (void)repeat; (void)gain; (void)quality; (void)stop_cb;
return new_voice();
}
void audio_mixer_stop(audio_mixer_voice_t *voice)
{
free(voice);
}
void audio_mixer_mix(float *buffer, size_t num_frames,
float gain_override, bool override)
{
(void)buffer; (void)num_frames; (void)gain_override; (void)override;
}
void audio_mixer_mix_s16(int16_t *buffer, size_t num_frames,
int32_t gain_override, bool override)
{
(void)buffer; (void)num_frames; (void)gain_override; (void)override;
}
void audio_mixer_voice_set_volume(audio_mixer_voice_t *voice, float val)
{
(void)voice; (void)val;
}
void audio_mixer_voice_set_avail(audio_mixer_voice_t *voice, size_t avail)
{
(void)voice; (void)avail;
}
void audio_mixer_sound_set_avail(audio_mixer_sound_t *sound, size_t avail)
{
(void)sound; (void)avail;
}
void audio_mixer_sound_set_data_owner(audio_mixer_sound_t *sound,
void *owner, void (*owner_free)(void *owner))
{
(void)sound; (void)owner; (void)owner_free;
}
size_t audio_mixer_voice_buffer_tell(audio_mixer_voice_t *voice)
{
(void)voice;
return 0;
}
bool audio_mixer_has_float_voices(void) { return true; }
bool audio_mixer_has_s16_voices(void) { return false; }
/* --- everything the locking paths must not reach --------------------- */
static void unreachable(const char *what)
{
fprintf(stderr, "stub %s reached unexpectedly\n", what);
abort();
}
bool command_event(enum event_command action, void *data)
{
(void)action; (void)data;
unreachable("command_event");
return false;
}
void *task_push_audio_mixer_load(const char *path,
retro_task_callback_t cb, void *user_data, bool system,
enum audio_mixer_slot_selection_type type, int slot)
{
(void)path; (void)cb; (void)user_data; (void)system; (void)type;
(void)slot;
unreachable("task_push_audio_mixer_load");
return NULL;
}
/* --- remaining frontend references ----------------------------------- */
#include "../../../list_special.h"
#include "../../../driver.h"
#include "../../../midi_driver.h"
#include "../../../file_path_special.h"
#include "../../../msg_hash.h"
#include "../../../audio/audio_thread_wrapper.h"
const char *char_list_new_special(enum string_list_type type, void *data)
{
(void)type; (void)data;
return NULL;
}
/* The real one walks the driver tables in retroarch.c; this harness
* includes audio_driver.c, so the audio table is visible here and the
* lookup is done against it, which is all this harness asks of it. */
int driver_find_index(const char *label, const char *drv)
{
int i;
(void)label;
for (i = 0; audio_drivers[i]; i++)
if (string_is_equal(audio_drivers[i]->ident, drv))
return i;
return -1;
}
bool midi_driver_synth_active(void) { return false; }
bool midi_driver_render_audio(float *out, size_t frames, unsigned rate)
{
(void)out; (void)frames; (void)rate;
return false;
}
const char *audio_thread_wrapped_ident(void *data)
{
(void)data;
return NULL;
}
const audio_driver_t *audio_thread_wrapped_driver(void *data)
{
(void)data;
return NULL;
}
bool audio_init_thread(const audio_driver_t **out_driver, void **out_data,
const char *device, unsigned out_rate, unsigned *new_rate,
unsigned latency, unsigned block_frames, bool raise_priority,
bool prefer_fast_cores, const audio_driver_t *driver)
{
(void)out_driver; (void)out_data; (void)device; (void)out_rate;
(void)new_rate; (void)latency; (void)block_frames;
(void)raise_priority; (void)prefer_fast_cores; (void)driver;
unreachable("audio_init_thread");
return false;
}
size_t fill_pathname_application_special(char *s, size_t len,
enum application_special_type type)
{
(void)type;
if (len)
s[0] = '\0';
return 0;
}
const char *msg_hash_to_str(enum msg_hash_enums msg)
{
(void)msg;
return "";
}
/* retro_resampler_realloc: the real one is linked in. */