Files
RetroArch/tasks/task_database_info.c
T
LibretroAdmin fa39ac26d3 menu: abandon explore/dbinfo tasks at teardown instead of waiting for them
Menu deinit blocked the main thread on two task_queue_wait() calls
before freeing the explore state and the database info cache.  Both
are reachable while the user is looking at the menu, and the work
being waited on is unbounded: a database info scan is a full
libretro-db query, seconds of frozen UI on slow storage, and on
Android an ANR.

Neither wait was protecting a worker mid-write into the state being
freed - the reason such waits usually exist.  Both tasks build their
result into their own handle and install it only from their
main-thread callback: menu_explore_build_list touches neither the
global explore state nor the icons (those load in the callback, on
the main thread), and the database scan writes only its own
db_list.  What the waits actually prevented was a LATE callback -
one arriving after teardown - installing into state that had just
been freed, which for explore means loading icons through a
torn-down video driver.

That is a stale-completion problem, and the tree already has the
answer for it: the generation token menu_dirwalk uses.  Teardown now
calls menu_explore_cancel_init_task() and menu_dbinfo_cancel_task(),
which bump a generation and flag the in-flight task cancelled; a
completion carrying an older generation installs nothing and lets
the task's own cleanup free what it built.  The main thread returns
immediately.

Two details worth naming, both found while checking rather than
assumed:

- The queue runs a task's callback whether or not it was cancelled,
  so a stale completion does reach the callback.  It must not clear
  the slot or the pending flag belonging to a NEWER task pushed
  after the cancel - for dbinfo that would drop the single-scan
  invariant and let two scans fight over the one cache slot.  Both
  callbacks now clear only when the retiring task is still the one
  owning the slot.
- While a cancelled explore task drains, the find-based
  in-progress check still reports true, so a fresh initialisation
  pushed in that window is refused.  This self-heals: the push sits
  in the explore displaylist build, which re-runs every rebuild
  while showing the "initialising" placeholder, and succeeds once
  the old task drains.

A CI guard keeps the property: task_queue_wait() must not appear
under menu/ or in the two task translation units the menu drives.
Its scope is deliberately narrow - the remaining wait sites (core
deinit save/load, NAT traversal, netplay MITM) are separate work,
and widening the guard is what should happen as each is converted.

Verified: strict C89 gates on both task TUs; full Linux build links
with HAVE_QT=1; the dirwalk, playlist and scan oracles all still
pass, the scan one under TSan as well; the guard was falsified by
reintroducing a wait on the deinit path, which it catches.
2026-08-15 07:34:03 +02:00

304 lines
9.3 KiB
C

/* RetroArch - A frontend for libretro.
* Copyright (C) 2011-2021 - Daniel De Matteis
*
* RetroArch is free software: you can redistribute it and/or modify it under the terms
* of the GNU General Public License as published by the Free Software Found-
* ation, either version 3 of the License, or (at your option) any later version.
*
* RetroArch is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
* without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
* PURPOSE. See the GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along with RetroArch.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <stdlib.h>
#include <string.h>
#include <string/stdstring.h>
#include "tasks_internal.h"
#include "../database_info.h"
#include "../menu/menu_driver.h"
/* Loading a database view means walking every record of the .rdb
* sequentially (libretrodb queries scan the whole file), which for
* large databases is many megabytes of IO - far too much to run on
* the menu thread during displaylist building. The displaylist
* parsers instead consult a one-slot result cache; on a miss they
* push this task, display a 'initialising list' placeholder, and
* get refreshed by the task callback once the scan (performed on
* the task queue) has finished and populated the cache.
*
* The cache retains ownership of the list. It holds the most
* recently requested {path, query} result, so refresh-driven
* rebuilds of the same view (and re-entering it) are free, and is
* replaced when a different view is requested. */
typedef struct db_info_handle
{
database_info_list_t *db_list;
char *path;
char *query; /* NULL = unfiltered */
unsigned generation; /* stale-completion guard */
} db_info_handle_t;
static database_info_list_t *db_info_cache_list = NULL;
static char *db_info_cache_path = NULL;
static char *db_info_cache_query = NULL;
/* True from task push until the task's main-thread callback has
* run. Deliberately NOT derived from task_queue_find(): the
* threaded queue has a window where the worker has finished (task
* no longer in the running list) but the callback has not yet
* executed. A find-based in-progress check reports false there,
* which lets a displaylist rebuild push a duplicate scan. The flag
* transitions strictly on the main thread (push / callback), so no
* such window exists. (Teardown no longer depends on this flag at
* all: it abandons the scan outright - see
* menu_dbinfo_cancel_task.) */
static bool db_info_task_pending = false;
/* Bumped whenever an in-flight scan is abandoned (menu teardown).
* A completion carrying an older generation hands nothing to the
* cache: the menu it was scanning for is gone. This is what lets
* teardown abandon the task rather than block the main thread until
* the scan finishes - the hazard the pending flag above describes
* (a callback repopulating the cache after deinit freed it) is
* closed by the generation, not by waiting. */
static unsigned db_info_generation;
static retro_task_t *db_info_task;
static bool db_info_key_equal(const char *a, const char *b)
{
return string_is_equal(a ? a : "", b ? b : "");
}
/* True while the cache slot holds a result - including a NULL
* result from a failed scan - for this exact {path, query}. Lets
* the displaylist parsers distinguish 'scan failed' (show the
* empty fallback, do NOT re-push) from 'never scanned' (push). */
static bool db_info_cache_occupied = false;
bool menu_dbinfo_cache_has(const char *path, const char *query)
{
if (!db_info_cache_occupied)
return false;
if (!db_info_key_equal(path, db_info_cache_path))
return false;
if (!db_info_key_equal(query, db_info_cache_query))
return false;
return true;
}
database_info_list_t *menu_dbinfo_cache_get(const char *path,
const char *query)
{
if (!menu_dbinfo_cache_has(path, query))
return NULL;
return db_info_cache_list;
}
void menu_dbinfo_cache_free(void)
{
if (db_info_cache_list)
{
database_info_list_free(db_info_cache_list);
free(db_info_cache_list);
db_info_cache_list = NULL;
}
if (db_info_cache_path)
{
free(db_info_cache_path);
db_info_cache_path = NULL;
}
if (db_info_cache_query)
{
free(db_info_cache_query);
db_info_cache_query = NULL;
}
db_info_cache_occupied = false;
}
static void free_db_info_handle(db_info_handle_t *db)
{
if (!db)
return;
if (db->path)
{
free(db->path);
db->path = NULL;
}
if (db->query)
{
free(db->query);
db->query = NULL;
}
if (db->db_list)
{
database_info_list_free(db->db_list);
free(db->db_list);
db->db_list = NULL;
}
free(db);
}
static void task_database_info_free(retro_task_t *task)
{
if (!task)
return;
free_db_info_handle((db_info_handle_t*)task->state);
}
static void task_database_info_handler(retro_task_t *task)
{
if (task)
{
db_info_handle_t *db = NULL;
if ((db = (db_info_handle_t*)task->state))
{
uint8_t flg = task_get_flags(task);
if (!((flg & RETRO_TASK_FLG_CANCELLED) > 0))
{
db->db_list = database_info_list_new(db->path,
(db->query && *db->query) ? db->query : NULL);
task_set_progress(task, 100);
}
}
task_set_flags(task, RETRO_TASK_FLG_FINISHED, true);
}
}
static void cb_task_database_info(
retro_task_t *task, void *task_data,
void *user_data, const char *err)
{
const char *stack_path = NULL;
db_info_handle_t *db = NULL;
struct menu_state *menu_st = menu_state_get_ptr();
if (!task)
return;
/* The pipeline is free again - but only if this callback belongs
* to the scan currently occupying it. A stale completion
* arriving after teardown pushed a newer scan must not clear the
* newer scan's pending flag, which would let a second concurrent
* scan start and fight over the single cache slot. */
if (db_info_task == task)
{
db_info_task = NULL;
db_info_task_pending = false;
}
if (!(db = (db_info_handle_t*)task->state))
return;
/* A scan that outlived the menu it was for: hand nothing over.
* The result stays on the handle and the task's own cleanup
* frees it. */
if (db->generation != db_info_generation)
return;
/* Hand the result (which may be NULL on scan failure - cached
* anyway, so the displaylist shows its empty-list fallback
* instead of re-pushing the task forever) over to the cache. */
menu_dbinfo_cache_free();
db_info_cache_list = db->db_list;
db_info_cache_path = db->path;
db_info_cache_query = db->query;
db_info_cache_occupied = true;
db->db_list = NULL;
db->path = NULL;
db->query = NULL;
/* If a database view for this path is currently displayed, it
* must be refreshed so it rebuilds and consumes the cache. If
* the user has already navigated elsewhere, leave the result
* cached for when they return. */
menu_entries_get_last_stack(&stack_path, NULL, NULL, NULL, NULL);
if ( stack_path && db_info_cache_path
&& string_is_equal(stack_path, db_info_cache_path))
menu_st->flags |= MENU_ST_FLAG_ENTRIES_NEED_REFRESH
| MENU_ST_FLAG_PREVENT_POPULATE;
}
bool menu_dbinfo_load_in_progress(void *data)
{
return db_info_task_pending;
}
/* Abandon any in-flight database info scan.
*
* Replaces waiting for it: the handler builds its result into the
* task's own handle and touches no menu state, so nothing needs the
* worker to have stopped before the cache is freed. The generation
* bump makes a completion already in flight hand nothing over. */
void menu_dbinfo_cancel_task(void)
{
db_info_generation++;
if (db_info_task)
{
task_set_flags(db_info_task, RETRO_TASK_FLG_CANCELLED, true);
db_info_task = NULL;
}
/* The pipeline is free again: a later request must be able to
* push a fresh scan. */
db_info_task_pending = false;
}
bool task_push_dbinfo_load(const char *path, const char *query)
{
retro_task_t *task = NULL;
db_info_handle_t *db = NULL;
if (!path || !*path)
goto error;
task = task_init();
db = (db_info_handle_t*)calloc(1, sizeof(db_info_handle_t));
if (!task || !db)
goto error;
/* One database scan at a time - the cache has a single slot,
* and a second concurrent scan would just fight over it. */
if (db_info_task_pending)
goto error;
db->db_list = NULL;
db->path = strdup(path);
db->query = (query && *query) ? strdup(query) : NULL;
db->generation = db_info_generation;
/* Silent task: no title, no notifications */
task->handler = task_database_info_handler;
task->state = db;
task->title = NULL;
task->progress = 0;
task->callback = cb_task_database_info;
task->cleanup = task_database_info_free;
task->flags |= RETRO_TASK_FLG_MUTE;
db_info_task_pending = true;
db_info_task = task;
task_queue_push(task);
return true;
error:
if (task)
{
free(task);
task = NULL;
}
free_db_info_handle(db);
return false;
}