From 85f8fd310e505dad2d97066782cf6fed0b6daebc Mon Sep 17 00:00:00 2001 From: LibretroAdmin Date: Wed, 16 Sep 2026 08:02:17 +0000 Subject: [PATCH] retroarch: the exit path drains the task queue before teardown The discovery pass's shutdown-ordering thread, opened by the mixer retirement fix: main_exit() tore drivers down and freed global state before task_queue_deinit(), and deinit abandons whatever is still queued - the worker breaks out mid-queue (its own comment asked whether it should), so remaining handlers never run, finish callbacks never fire, and cleanup callbacks never release their owners. A mixer load, a screenshot, a database scan chunk in flight at quit just leaked, and anything that did retire in that stretch retired into torn subsystems - the class the MIXER_INITED latch guards one door of. main_exit() now runs retroarch_drain_tasks_for_exit() first: task_queue_reset() cancels everything so cancellation-aware handlers wind down instead of completing their work, and a bounded task_queue_wait_timeout() retires the queue - handlers, finish callbacks and cleanups all run on the exiting thread while every subsystem they can reach is still alive. The bound (three seconds) keeps a stuck handler from hanging quit; whatever survives it meets the old abandonment, demoted from rule to fallback. Tasks scheduled for later (a set 'when') do not hold the wait and are dropped as designed. The worker's questioning comment now states the contract. samples/tasks/exit_drain links the shipping objects with only main() replaced, boots headless with the threaded task runner, and puts the exit-time population in flight: a cancellation-aware handler mid-run, a task completing on its own, and one scheduled thirty seconds out. The drain must retire the first two - handler wound down, finish and cleanup callbacks run - in well under the bound (432 microseconds measured) without the deferred task holding it. The MODE=old lane runs the pre-change order, straight to task_queue_deinit(): the aware task's callbacks never run, which is the abandonment this change demotes. Verified: both harness lanes green as stated, playlist_nav green, thread-read audit 0 findings / 0 allowlisted pairs, C89 gate clean on task_queue.c, full build scanned clean for the warning classes, hot-function budgets and the 74-lane compile matrix. --- libretro-common/queues/task_queue.c | 8 +- retroarch.c | 18 ++ retroarch.h | 5 + samples/tasks/exit_drain/Makefile | 10 + samples/tasks/exit_drain/build.sh | 71 ++++++ samples/tasks/exit_drain/exit_drain_test.c | 252 +++++++++++++++++++++ 6 files changed, 363 insertions(+), 1 deletion(-) create mode 100644 samples/tasks/exit_drain/Makefile create mode 100755 samples/tasks/exit_drain/build.sh create mode 100644 samples/tasks/exit_drain/exit_drain_test.c diff --git a/libretro-common/queues/task_queue.c b/libretro-common/queues/task_queue.c index e28beaf36c..113bad2d79 100644 --- a/libretro-common/queues/task_queue.c +++ b/libretro-common/queues/task_queue.c @@ -754,7 +754,13 @@ static void threaded_worker(void *userdata) if (!worker_continue) { slock_unlock(running_lock); - break; /* should we keep running until all tasks finished? */ + /* No: draining is the caller's job, done while the + * subsystems that finish callbacks reach are still alive + * (RetroArch's exit path cancels and drains, bounded, + * before any teardown). Whatever is still here when deinit + * runs is deliberately abandoned - running its handlers + * during teardown would be worse. */ + break; } /* Get first task to run */ diff --git a/retroarch.c b/retroarch.c index b70f33677d..102439cb0a 100644 --- a/retroarch.c +++ b/retroarch.c @@ -6393,6 +6393,22 @@ static void sdl_exit(void) * Cleanly exit RetroArch. * **/ +/* Retire the task queue while every subsystem a finish callback can + * reach is still alive: the exit path tears drivers down before + * task_queue_deinit(), and deinit abandons whatever is still queued + * - handlers never run, finish callbacks never fire, cleanup + * callbacks never release their owners. Cancelling first lets + * cancellation-aware handlers wind down instead of completing their + * work, and the bound keeps a stuck transfer from hanging quit; + * whatever survives the bound meets the old abandonment, now the + * fallback instead of the rule. Scheduled-for-later tasks (a set + * 'when') do not hold the wait and are dropped as designed. */ +void retroarch_drain_tasks_for_exit(void) +{ + task_queue_reset(); + task_queue_wait_timeout(NULL, NULL, 3 * 1000 * 1000); +} + void main_exit(void *args) { struct rarch_state *p_rarch = &rarch_st; @@ -6402,6 +6418,8 @@ void main_exit(void *args) #endif settings_t *settings = config_get_ptr(); + retroarch_drain_tasks_for_exit(); + video_driver_restore_cached(settings); #if defined(HAVE_GFX_WIDGETS) diff --git a/retroarch.h b/retroarch.h index 2ccdc2109a..fcbe97716f 100644 --- a/retroarch.h +++ b/retroarch.h @@ -127,6 +127,11 @@ bool retroarch_override_setting_is_set(enum rarch_override_setting enum_idx, voi const char* video_shader_get_current_shader_preset(void); +/* Cancels and drains the task queue (bounded) so in-flight tasks + * retire while the subsystems their callbacks reach are alive. + * main_exit() runs it first; exposed for the exit-drain harness. */ +void retroarch_drain_tasks_for_exit(void); + /** * retroarch_main_init: * @argc : Count of (commandline) arguments. diff --git a/samples/tasks/exit_drain/Makefile b/samples/tasks/exit_drain/Makefile new file mode 100644 index 0000000000..7a1cdd22cf --- /dev/null +++ b/samples/tasks/exit_drain/Makefile @@ -0,0 +1,10 @@ +TARGET := exit_drain_test + +all: + @sh ./build.sh + +clean: + rm -f $(TARGET) retroarch_nomain.o retroarch_nomain.d \ + harness_main.o harness_main.d + +.PHONY: all clean diff --git a/samples/tasks/exit_drain/build.sh b/samples/tasks/exit_drain/build.sh new file mode 100755 index 0000000000..0d67649d30 --- /dev/null +++ b/samples/tasks/exit_drain/build.sh @@ -0,0 +1,71 @@ +#!/bin/sh +# Builds the exit-path task-drain harness. +# +# Like playlist_nav, this links the SHIPPING objects of a built +# RetroArch with only main() replaced - nothing is stubbed. The seam +# under test runs through rgui_frame's aspect-change detection, the staged config on the +# rgui handle, and the application rgui_render performs on the main +# thread before CMD_EVENT_VIDEO_SET_ASPECT_RATIO fires. +# real frame and render code can see whether the write lands in the +# configuration or dies with the frame's snapshot copy. +# +# Requires a completed NON-Qt build: with Qt enabled main() lives in +# ui_qt.o, which drags the whole Qt UI in with it. From the repo +# root: +# +# ./configure --disable-qt && make +# samples/tasks/exit_drain/build.sh +# +# retroarch.c is recompiled here with main renamed, so the harness can +# supply its own without having to exclude the object that holds most +# of the frontend. Both the compile and the link reuse the project's +# own command lines, taken from make -n, so the harness cannot drift +# away from how the program is really built. +set -eu + +here=$(cd "$(dirname "$0")" && pwd) +root=$(cd "$here/../../.." && pwd) + +cd "$root" + +if [ ! -f obj-unix/release/retroarch.o ]; then + echo "build RetroArch first: ./configure --disable-qt && make" >&2 + exit 1 +fi + +cc_line=$(mktemp) +ld_line=$(mktemp) +trap 'rm -f "$cc_line" "$ld_line"' EXIT + +# The project's own compile line for retroarch.o ... +touch retroarch.c +make -n 2>/dev/null | grep -E '\-o obj-unix/release/retroarch\.o' | head -1 > "$cc_line" +if [ ! -s "$cc_line" ]; then + echo "could not determine the compile command for retroarch.o" >&2 + exit 1 +fi + +# ... reused twice: once for retroarch.c with main renamed out of the +# way, once for the harness itself, so both are built exactly the way +# the frontend is. +sed 's#-o obj-unix/release/retroarch\.o#-Dmain=rarch_harness_unused_main -o samples/tasks/exit_drain/retroarch_nomain.o#' \ + "$cc_line" | sh + +sed -e 's#-o obj-unix/release/retroarch\.o#-o samples/tasks/exit_drain/harness_main.o#' \ + -e 's# retroarch\.c# samples/tasks/exit_drain/exit_drain_test.c#' \ + "$cc_line" | sh + +# The project's own link line, with retroarch.o swapped for the pair +# above. Removing the binary first is what makes make -n emit it. +rm -f retroarch +make -n 2>/dev/null | grep -E ' -o retroarch ' | tail -1 > "$ld_line" +if [ ! -s "$ld_line" ]; then + echo "could not determine the link command" >&2 + exit 1 +fi + +sed -e 's#obj-unix/release/retroarch\.o#samples/tasks/exit_drain/retroarch_nomain.o samples/tasks/exit_drain/harness_main.o#' \ + -e 's#-o retroarch #-o samples/tasks/exit_drain/exit_drain_test #' \ + "$ld_line" | sh + +echo "built samples/tasks/exit_drain/exit_drain_test" diff --git a/samples/tasks/exit_drain/exit_drain_test.c b/samples/tasks/exit_drain/exit_drain_test.c new file mode 100644 index 0000000000..445e169b25 --- /dev/null +++ b/samples/tasks/exit_drain/exit_drain_test.c @@ -0,0 +1,252 @@ +/* In-flight tasks retire before the exit path tears anything down. + * + * Links the shipping RetroArch objects with only main() replaced and + * drives retroarch_drain_tasks_for_exit() - the first thing + * main_exit() now runs - against the task queue's own machinery: + * task_queue_deinit() abandons whatever is still queued (the worker + * breaks out mid-queue; nothing runs remaining handlers, finish + * callbacks, or cleanup callbacks), and the exit path used to reach + * it only after the drivers were gone. + * + * Lanes: + * - a cancellation-aware task (its handler winds down when it sees + * RETRO_TASK_FLG_CANCELLED) must have its finish and cleanup + * callbacks run by the drain; + * - a task that completes on its own before the drain must retire + * through the same gather; + * - a scheduled-for-later task (a 'when' in the future) must not + * hold the wait: the drain returns promptly and that task meets + * the old abandonment, as designed. + * + * The old-order lane (MODE=old) skips the drain and goes straight + * to task_queue_deinit() the way the exit path used to: the + * cancellation-aware task's callbacks never run, which is the + * abandonment this change demotes from rule to fallback. + * + * Requires a completed non-Qt build: + * + * ./configure --disable-qt && make + * samples/tasks/exit_drain/build.sh + */ + +#include +#include +#include +#include + +#include +#include