diff --git a/games/NXDoom/src/doom/d_main.c b/games/NXDoom/src/doom/d_main.c index 52df6eda1..b715a6283 100644 --- a/games/NXDoom/src/doom/d_main.c +++ b/games/NXDoom/src/doom/d_main.c @@ -1294,6 +1294,12 @@ void d_doomloop(void) while (1) { + /* Safe point (outside any framebuffer/heap access) for nxstore's + * SIGTERM-driven close request to actually take effect - see + * i_install_quit_signal() in i_system.h. + */ + + i_poll_quit_signal(); d_run_frame(); } } diff --git a/games/NXDoom/src/i_main.c b/games/NXDoom/src/i_main.c index bd9dab609..17029afc5 100644 --- a/games/NXDoom/src/i_main.c +++ b/games/NXDoom/src/i_main.c @@ -57,6 +57,15 @@ void d_doom_main(void); int main(int argc, char **argv) { + /* Lets nxstore (or any other supervisor) ask this process to exit + * cleanly via SIGTERM instead of the only other option being a forced + * task_delete() from outside - see i_system.h/i_system.c for why that + * matters on this board (a forced kill mid framebuffer/heap access was + * observed to hang the whole system, not just this task). + */ + + i_install_quit_signal(); + /* save arguments */ myargc = argc; diff --git a/games/NXDoom/src/i_system.c b/games/NXDoom/src/i_system.c index b3867e0ad..a98c16a17 100644 --- a/games/NXDoom/src/i_system.c +++ b/games/NXDoom/src/i_system.c @@ -22,10 +22,13 @@ * Included Files ****************************************************************************/ +#include +#include #include #include #include #include +#include #include #include "config.h" @@ -75,6 +78,15 @@ static atexit_listentry_t *exit_funcs = NULL; static boolean already_quitting = false; +/* Set only by i_quit_signal_handler() (async-signal-safe: a single + * sig_atomic_t store, nothing else) and read only by + * i_poll_quit_signal(), called from a safe point in the main loop - see + * the comment on i_install_quit_signal() in i_system.h for why the + * actual i_quit() cleanup is deferred out of the signal handler itself. + */ + +static volatile sig_atomic_t quit_requested = 0; + /* Read Access Violation emulation. * * From PrBoom+, by entryway. @@ -320,6 +332,71 @@ void i_quit(void) exit(0); } +/* i_quit_signal_handler + * + * A supervisor process (nxstore) has no reachable in-game quit path to + * drive (no keyboard/touch input is wired up here) - it can only ask + * from the outside, via SIGTERM. This handler does the one thing a + * signal handler is safe to do: set a flag. It must NOT call i_quit() + * (or anything it does - munmap, fclose, exit()'s atexit chain) directly, + * because a signal can land at literally any point in this process's own + * execution, including mid-malloc()/mid-blit - exactly the same "unsafe + * mid-operation teardown" risk as being force-killed from outside, just + * moved from another task's context into this one. i_poll_quit_signal() + * defers the real work to a known-safe boundary instead. + */ + +static void i_quit_signal_handler(int signo) +{ + (void)signo; + quit_requested = 1; +} + +void i_install_quit_signal(void) +{ + struct sigaction sa; + + /* This board's flat, single address-space build can relaunch NXDoom + * (via nxpkg) as a fresh loadable ELF module - a proper posix_spawn of + * a new module load, which gets its own zeroed .bss/re-initialized + * .data - but GAMES_NXDOOM is a tristate Kconfig symbol and can also + * be built in as a true built-in (MODULE=n) sharing this process's + * address space across "launches" with no fresh .bss at all. Reset + * both pieces of state a stale second invocation could see: a leaked + * quit_requested flag would call i_quit() again before the game even + * starts, and a leaked exit_funcs chain would run every previous + * invocation's exit handlers a second time (double free()s, etc.) in + * addition to this invocation's own. + */ + + quit_requested = 0; + exit_funcs = NULL; + + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = i_quit_signal_handler; + + if (sigaction(SIGTERM, &sa, NULL) < 0) + { + /* Not fatal - the game still runs, it just can't be asked to + * close cleanly from the outside (nxstore's close button will + * have nothing to signal into). Surface it rather than silently + * leaving close non-functional with no trace of why. + */ + + syslog(LOG_WARNING, + "nxdoom: failed to install SIGTERM handler: %d\n", errno); + } +} + +void i_poll_quit_signal(void) +{ + if (quit_requested) + { + syslog(LOG_WARNING, "nxdoom: quit signal seen, calling i_quit\n"); + i_quit(); + } +} + void i_error(const char *error, ...) { char msgbuf[512]; diff --git a/games/NXDoom/src/i_system.h b/games/NXDoom/src/i_system.h index ae1c56806..ac0c934ae 100644 --- a/games/NXDoom/src/i_system.h +++ b/games/NXDoom/src/i_system.h @@ -73,6 +73,23 @@ ticcmd_t *i_base_ticcmd(void); void i_quit(void) NORETURN; +/* Installs a SIGTERM handler that only sets a flag (async-signal-safe) - + * the actual i_quit() cleanup (unmapping the framebuffer, closing fds) + * runs later from i_poll_quit_signal(), called once per tic from a known + * safe point in the main loop rather than from the signal handler itself, + * so a supervisor process (nxstore) requesting an exit can never land in + * the middle of a frame's worth of direct framebuffer/heap access. + */ + +void i_install_quit_signal(void); + +/* Checks the flag set by the SIGTERM handler and calls i_quit() if it's + * set. Must only be called from a safe point in the main loop - see + * i_install_quit_signal(). + */ + +void i_poll_quit_signal(void); + void i_error(const char *error, ...) NORETURN PRINTF_ATTR(1, 2); void i_tactile(int on, int off, int total);