Skip to content

Commit 4666d70

Browse files
committed
command/retroarch: clarify lifecycle comments around
reinit and core deinit Two minor doc cleanups - no behavioral change. command.c: the comment on the cached_snapshot static buffer in command_event_reinit claimed the buffer is 'freed at shutdown', but no shutdown free path exists. Update the comment to describe what actually happens: the static is not freed, but it's bounded at one framebuffer's worth of memory and reclaimed by the OS at process exit. Note explicitly that adding a teardown hook would require wiring the static into retroarch_deinit_drivers and that we deliberately don't. retroarch.c: the CMD_EVENT_CORE_DEINIT handler recursively calls command_event(CMD_EVENT_UNPAUSE, NULL) at its top, re-entering the same dispatcher. The current UNPAUSE branch is small enough that the self-call is safe, but the safety constraint isn't documented. Add a comment so future changes to UNPAUSE that would touch core state get noticed during review (we're mid-deinit; touching core state from there would be a UAF).
1 parent 9bfa19c commit 4666d70

2 files changed

Lines changed: 13 additions & 3 deletions

File tree

command.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2521,8 +2521,13 @@ void command_event_reinit(const int flags)
25212521
/* Restore the snapshot and ask the new driver to replay it so the
25222522
* paused-core background appears in the first post-reinit frame.
25232523
* The buffer stays live across subsequent frame_cb calls from the
2524-
* core (which overwrite the pointer) and is either reused on the
2525-
* next reinit or freed at shutdown. */
2524+
* core (which overwrite the pointer) and is reused on the next
2525+
* reinit. The static cached_snapshot itself is not freed at
2526+
* shutdown - it's a one-shot leak bounded at one framebuffer's
2527+
* worth of memory, reclaimed by the OS on process exit. Adding
2528+
* a teardown hook would mean wiring command_event_reinit's
2529+
* statics into retroarch_deinit_drivers; the size cap makes the
2530+
* leak benign in practice, so we leave it. */
25262531
if (cached_snapshot_p && cached_snapshot_h)
25272532
{
25282533
video_st->frame_cache_data = cached_snapshot;

retroarch.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4362,7 +4362,12 @@ bool command_event(enum event_command cmd, void *data)
43624362
*video_st = video_state_get_ptr();
43634363
rarch_system_info_t *sys_info = &runloop_st->system;
43644364

4365-
/* Restore unpaused state */
4365+
/* Restore unpaused state. The recursive command_event call
4366+
* here re-enters this dispatcher; the UNPAUSE branch is
4367+
* deliberately small (clears flags, resumes audio) and
4368+
* does not touch core state, so the self-call is safe.
4369+
* Any future addition to the UNPAUSE handler that would
4370+
* touch core state must consider that we're mid-deinit. */
43664371
runloop_st->paused_hotkey = false;
43674372
command_event(CMD_EVENT_UNPAUSE, NULL);
43684373

0 commit comments

Comments
 (0)