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

@@ -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 */

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)

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.

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

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"

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 <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <boolean.h>
+#include <time/rtime.h>
+#include <file/config_file.h>
+#include <streams/file_stream.h>
+#include <queues/task_queue.h>
+#include <features/features_cpu.h>
+#include <retro_timers.h>
+
+#include "../../../configuration.h"
+#include "../../../retroarch.h"
+#include "../../../frontend/frontend_driver.h"
+
+static unsigned failures = 0;
+
+#define CHECK(cond, ...) \
+   do { \
+      if (!(cond)) \
+      { \
+         fprintf(stderr, "FAIL %s:%d: ", __FILE__, __LINE__); \
+         fprintf(stderr, __VA_ARGS__); \
+         fprintf(stderr, "\n"); \
+         failures++; \
+      } \
+   } while (0)
+
+/* Witnesses, one per lane. */
+static volatile int aware_finished   = 0;
+static volatile int aware_callback   = 0;
+static volatile int aware_cleanup    = 0;
+static volatile int quick_callback   = 0;
+static volatile int deferred_touched = 0;
+
+/* Cancellation-aware: polls its flag between chunks, like a
+ * transfer. Capped so the old-order lane - which never cancels -
+ * models a long task rather than an unbounded one: deinit joins
+ * the worker mid-handler, and an uncappable spin would turn the
+ * abandonment being demonstrated into a hang of the harness. */
+static void aware_handler(retro_task_t *task)
+{
+   int spins = 0;
+   while (   !(task_get_flags(task) & RETRO_TASK_FLG_CANCELLED)
+          && spins++ < 8000)
+      retro_sleep(1);
+   aware_finished = 1;
+   task_set_flags(task, RETRO_TASK_FLG_FINISHED, true);
+}
+
+static void aware_cb(retro_task_t *task, void *task_data,
+      void *user_data, const char *error)
+{
+   (void)task; (void)task_data; (void)user_data; (void)error;
+   aware_callback = 1;
+}
+
+static void aware_cleanup_cb(retro_task_t *task)
+{
+   (void)task;
+   aware_cleanup = 1;
+}
+
+/* Completes on its own, immediately. */
+static void quick_handler(retro_task_t *task)
+{
+   task_set_flags(task, RETRO_TASK_FLG_FINISHED, true);
+}
+
+static void quick_cb(retro_task_t *task, void *task_data,
+      void *user_data, const char *error)
+{
+   (void)task; (void)task_data; (void)user_data; (void)error;
+   quick_callback = 1;
+}
+
+/* Scheduled for later: must not hold the drain. */
+static void deferred_handler(retro_task_t *task)
+{
+   deferred_touched = 1;
+   task_set_flags(task, RETRO_TASK_FLG_FINISHED, true);
+}
+
+static retro_task_t *make_task(retro_task_handler_t handler,
+      retro_task_callback_t cb, retro_task_handler_t cleanup)
+{
+   retro_task_t *t = task_init();
+   if (!t)
+      return NULL;
+   t->handler  = handler;
+   t->callback = cb;
+   t->cleanup  = cleanup;
+   return t;
+}
+
+int main(int argc, char *argv[])
+{
+   char fixture_dir[512];
+   char cmd[700];
+   static char cfg_path[640];
+   char *rarch_argv[8];
+   int rarch_argc  = 0;
+   bool old_order  = false;
+   retro_task_t *t;
+   retro_time_t t0;
+   retro_time_t drain_us = 0;
+
+   (void)argc;
+   (void)argv;
+
+   {
+      const char *mode = getenv("MODE");
+      old_order = (mode && strcmp(mode, "old") == 0);
+   }
+
+   snprintf(fixture_dir, sizeof(fixture_dir),
+         "/tmp/exit_drain_%ld", (long)getpid());
+   snprintf(cmd, sizeof(cmd), "mkdir -p %s", fixture_dir);
+   if (system(cmd) != 0)
+      return 1;
+
+   rarch_argv[rarch_argc++] = (char*)"retroarch";
+   rarch_argv[rarch_argc++] = (char*)"--menu";
+   rarch_argv[rarch_argc++] = (char*)"--config";
+   rarch_argv[rarch_argc++] = cfg_path;
+
+   config_file_set_io_default(config_file_io_filestream());
+   rtime_init();
+   retroarch_config_init();
+   retroarch_ctl(RARCH_CTL_STATE_FREE, NULL);
+   frontend_driver_init_first(NULL);
+   {
+      FILE *cfg;
+      snprintf(cfg_path, sizeof(cfg_path), "%s/harness.cfg",
+            fixture_dir);
+      if ((cfg = fopen(cfg_path, "wb")))
+      {
+         fprintf(cfg, "video_driver = \"null\"\n");
+         fprintf(cfg, "audio_driver = \"null\"\n");
+         fprintf(cfg, "input_driver = \"null\"\n");
+         fprintf(cfg, "input_joypad_driver = \"null\"\n");
+         fprintf(cfg, "menu_driver = \"rgui\"\n");
+         fprintf(cfg, "threaded_data_runloop_enable = \"true\"\n");
+         fclose(cfg);
+      }
+   }
+
+   if (!retroarch_main_init(rarch_argc, rarch_argv))
+   {
+      fprintf(stderr, "FAIL: retroarch_main_init failed\n");
+      return 1;
+   }
+
+   /* The in-flight population at "quit": one cancellation-aware,
+    * one already finishing, one scheduled for later. */
+   t = make_task(aware_handler, aware_cb, aware_cleanup_cb);
+   CHECK(t != NULL, "fixture: task_init failed");
+   if (t) task_queue_push(t);
+
+   t = make_task(quick_handler, quick_cb, NULL);
+   if (t) task_queue_push(t);
+
+   t = make_task(deferred_handler, NULL, NULL);
+   if (t)
+   {
+      t->when = cpu_features_get_time_usec() + 30 * 1000 * 1000;
+      task_queue_push(t);
+   }
+
+   /* Let the worker pick the aware task up so cancellation lands on
+    * a running handler, the harder case. */
+   retro_sleep(50);
+
+   if (!old_order)
+   {
+      t0 = cpu_features_get_time_usec();
+      retroarch_drain_tasks_for_exit();
+      drain_us = cpu_features_get_time_usec() - t0;
+
+      CHECK(aware_finished == 1,
+            "the cancellation-aware handler never wound down");
+      CHECK(aware_callback == 1,
+            "the aware task's finish callback did not run");
+      CHECK(aware_cleanup == 1,
+            "the aware task's cleanup callback did not run");
+      CHECK(quick_callback == 1,
+            "the completing task's callback did not run");
+      CHECK(deferred_touched == 0,
+            "the scheduled-for-later task ran during the drain");
+      CHECK(drain_us < 2 * 1000 * 1000,
+            "a deferred 'when' held the drain (%ld us)",
+            (long)drain_us);
+   }
+
+   /* The rest of the old exit order; on both paths the deferred
+    * task meets abandonment here, and on the old path everything
+    * does. */
+   task_queue_deinit();
+
+   if (old_order)
+   {
+      CHECK(aware_callback == 0 && aware_cleanup == 0,
+            "old order: expected abandonment, but callbacks ran");
+      printf("exit_drain: old order abandons, as documented\n");
+   }
+   else
+      printf("exit_drain: all lanes passed (drain %ld us)\n",
+            (long)drain_us);
+
+   snprintf(cmd, sizeof(cmd), "rm -rf %s", fixture_dir);
+   if (system(cmd) != 0) { /* fixture dir left behind; harmless */ }
+
+   if (failures)
+   {
+      fprintf(stderr, "FAILURES (%u)\n", failures);
+      return 1;
+   }
+   return 0;
+}