cheevos: make queued_command atomic to fix bg-thread /

main-thread race

rcheevos_locals.queued_command was a plain enum event_command,
written from background threads (the rcheevos client's HTTP
completion callback at the bottom of
rcheevos_client_load_game_callback, and the rewind init/deinit
toggles inside rcheevos_apply_hardcore_state) and read on the
main thread by rcheevos_test once per frame.

Concurrent plain access to a non-atomic field is undefined: there
is no memory barrier publishing the writer's prior writes, no
guarantee that the reader observes the value at all on weak-memory
targets, and on architectures where enum is wider than the
naturally-atomic store size, a torn read.

Replace the field type with retro_atomic_int_t. Writers go through
retro_atomic_store_release_int; the reader in rcheevos_test takes
a single acquire-load snapshot at the top of the dispatch block,
preventing the three previous reads (the !=NONE check, the
==FINALIZE check, and the dispatch argument) from observing
different values if a writer fires between them.

Mirrors the atomic discipline already established for
gfx_thumbnail_t.status (gfx/gfx_thumbnail.c:50-65). The static
initializer for atomic_int with literal 0 is well-defined for all
retro_atomic.h backends (C11 stdatomic, C++11 std::atomic, and the
volatile-int fallback) - and CMD_EVENT_NONE happens to be 0.

This is the first of two patches addressing the queued_command
hazard. The second adds a load-generation counter to filter stale
bg-thread completions that race with rcheevos_unload, preventing
a finalize for game A from being applied to the memory map of
game B.

cheevos/cheevos.c

@@ -90,7 +90,9 @@ static rcheevos_locals_t rcheevos_locals =
    NULL, /* client */
    {{0}},/* memory */
 #ifdef HAVE_THREADS
-   CMD_EVENT_NONE, /* queued_command */
+   /* queued_command (atomic). CMD_EVENT_NONE == 0; static
+    * zero-initialization satisfies all retro_atomic.h backends. */
+   0,
 #endif
    "",   /* user_agent_prefix */
    "",   /* user_agent_core */
@@ -812,7 +814,8 @@ bool rcheevos_unload(void)
    rc_client_unload_game(rcheevos_locals.client);
 
 #ifdef HAVE_THREADS
-   rcheevos_locals.queued_command = CMD_EVENT_NONE;
+   retro_atomic_store_release_int(&rcheevos_locals.queued_command,
+         CMD_EVENT_NONE);
 #endif
 
    if (rcheevos_locals.memory.count > 0)
@@ -836,7 +839,8 @@ bool rcheevos_unload(void)
    }
 
 #ifdef HAVE_THREADS
-   rcheevos_locals.queued_command = CMD_EVENT_NONE;
+   retro_atomic_store_release_int(&rcheevos_locals.queued_command,
+         CMD_EVENT_NONE);
 #endif
 
    if (!config_get_ptr()->arrays.cheevos_token[0])
@@ -919,7 +923,8 @@ static void rcheevos_toggle_hardcore_active(rcheevos_locals_t* locals)
             /* have to "schedule" this.
              * CMD_EVENT_REWIND_DEINIT should
              * only be called on the main thread */
-            rcheevos_locals.queued_command = CMD_EVENT_REWIND_DEINIT;
+            retro_atomic_store_release_int(&rcheevos_locals.queued_command,
+                  CMD_EVENT_REWIND_DEINIT);
          }
          else
 #endif
@@ -943,7 +948,8 @@ static void rcheevos_toggle_hardcore_active(rcheevos_locals_t* locals)
             /* have to "schedule" this.
              * CMD_EVENT_REWIND_INIT should
              * only be called on the main thread */
-            rcheevos_locals.queued_command = CMD_EVENT_REWIND_INIT;
+            retro_atomic_store_release_int(&rcheevos_locals.queued_command,
+                  CMD_EVENT_REWIND_INIT);
          }
          else
 #endif
@@ -1114,14 +1120,23 @@ Test all the achievements (call once per frame).
 void rcheevos_test(void)
 {
 #ifdef HAVE_THREADS
-   if (rcheevos_locals.queued_command != CMD_EVENT_NONE)
+   /* Snapshot the queued command once with an acquire-load. The
+    * matching release-stores are at the writer sites in this file
+    * (rcheevos_unload, the rewind toggles, and the bg-thread
+    * finalize at the bottom of rcheevos_client_load_game_callback).
+    * Without the snapshot the three reads at the previous
+    * !=NONE / ==FINALIZE / dispatch sites could each see a
+    * different value if a writer fires between them. */
+   int cmd = retro_atomic_load_acquire_int(&rcheevos_locals.queued_command);
+   if (cmd != CMD_EVENT_NONE)
    {
-      if (rcheevos_locals.queued_command == CMD_CHEEVOS_FINALIZE_LOAD)
+      if (cmd == CMD_CHEEVOS_FINALIZE_LOAD)
          rcheevos_finalize_game_load_on_ui_thread();
       else
-         command_event(rcheevos_locals.queued_command, NULL);
+         command_event((enum event_command)cmd, NULL);
 
-      rcheevos_locals.queued_command = CMD_EVENT_NONE;
+      retro_atomic_store_release_int(&rcheevos_locals.queued_command,
+            CMD_EVENT_NONE);
    }
 #endif
 
@@ -1733,7 +1748,8 @@ static void rcheevos_client_load_game_callback(int result,
    /* Have to "schedule" this. Game image should not be
     * loaded into memory on background thread */
    if (!task_is_on_main_thread())
-      rcheevos_locals.queued_command = (enum event_command)CMD_CHEEVOS_FINALIZE_LOAD;
+      retro_atomic_store_release_int(&rcheevos_locals.queued_command,
+            CMD_CHEEVOS_FINALIZE_LOAD);
    else
 #endif
       rcheevos_finalize_game_load_on_ui_thread();
@@ -1752,7 +1768,8 @@ bool rcheevos_load(const void *data)
       && settings->bools.cheevos_enable;
 
 #ifdef HAVE_THREADS
-   rcheevos_locals.queued_command = CMD_EVENT_NONE;
+   retro_atomic_store_release_int(&rcheevos_locals.queued_command,
+         CMD_EVENT_NONE);
 #endif
 
    /* If achievements are not enabled, or the core doesn't

cheevos/cheevos_locals.h

@@ -26,6 +26,7 @@
 
 #ifdef HAVE_THREADS
 #include <rthreads/rthreads.h>
+#include <retro_atomic.h>
 #endif
 
 #include <retro_common_api.h>
@@ -85,7 +86,13 @@ typedef struct rcheevos_locals_t
    rc_libretro_memory_regions_t memory;/* achievement addresses to core memory mappings */
 
 #ifdef HAVE_THREADS
-   enum event_command queued_command; /* action queued by background thread to be run on main thread */
+   /* Action queued by a background thread (e.g. the rcheevos
+    * client's HTTP completion thread) to be dispatched on the
+    * main thread by rcheevos_test. Atomic because writers can
+    * be on a worker thread while the main-thread reader is
+    * polling per frame; plain enum access would be a data race
+    * with no memory barrier. */
+   retro_atomic_int_t queued_command;
 #endif
 
    char user_agent_prefix[128];       /* RetroArch/OS version information */