sneak/rgoue
1
0
Fork 0
Code Issues 10 Pull Requests Actions Packages Projects Releases Wiki Activity

fix: take the signal-time autosave on the game goroutine (closes #24)

Browse Source 
The SIGHUP/SIGTERM handler gob-encoded the live game tree from the signal
goroutine while the game goroutine was mid-turn mutating it, and AutoSave
removed the save file before encoding — so the failure mode was not a
stale save but a deleted one followed by a possibly torn replacement,
with a window in which the player had neither. The suite has run under
-race since 2026-08-09 and was green because nothing had ever driven the
turn loop concurrently with a signal: evidence of untested, not of safe.

The handler no longer writes anything. AutoSaveOnSignal posts a request,
wakes the input read, and waits up to signalSaveTimeout for the game
goroutine to take it; the encode runs on the goroutine that owns the
state, at the three points where that goroutine can sit: between turns
(command), on waking from a blocked readchar, and while parked in the `!`
shell escape (runShellEscape, which now runs the shell on a helper
goroutine so a hangup during it still rescues the game).

Blocked on input is the case that matters — a dropped connection lands
while the player is thinking, so a flag checked only between turns would
never be looked at. Terminal.ReadChar therefore returns (byte, bool),
with ok false meaning "woken by Interrupt, no key", and term.Tcell posts
a tcell.EventInterrupt onto tcell's own event queue to unpark PollEvent.
readchar services the request and reads again, so no caller sees it.

Running the shell on a helper goroutine would also have moved
term.Tcell.ShellEscape's panic on a failed Screen.Resume onto it, and a
panic at the top of any goroutine terminates the process without running
the deferred calls of the others — including cmd/rogue/main.go's
`defer t.Fini()`. The tty would have been left raw on precisely the path
where the terminal is already broken, which is issue #12's failure on a
path this change created. runShellEscape therefore recovers the helper's
panic and re-raises it on the game goroutine, whose stack has the restore
in it, so "every path restores the terminal via Terminal.Fini before
exiting" stays true.

saveFile writes a temporary file in the save's own directory, fsyncs it
and renames it over the target instead of truncating in place, so a save
that fails — or never happens because the deadline ran out — leaves the
player's previous save whole.

What the handoff guarantees is stated exactly rather than flatteringly:
the encode runs on the state-owning goroutine, so the snapshot is
internally consistent and restorable, but it is not necessarily taken
between commands. Only the check at the top of command is; the other two
service points both sit inside a command call already under way. readchar
is reached from mid-command prompts (--More--, askOverwrite, getStr, the
direction and pack prompts) with the command's mutations already applied,
and runShellEscape is reached from shell, an ordinary '!' command handler
dispatched inside command, with that turn's DoDaemons(Before) and
DoFuses(Before) already fired and its AFTER pass not yet. Restoring
re-enters playit at the top of command, so either way the rest of that
command is lost and a fresh BEFORE pass runs on top of the one in the
snapshot.

The SIGINT/SIGQUIT no-save decision and the single-signal-read ordering
guarantee are untouched. pendingSaver reads the game out from under its
mutex rather than delegating with it held, because the delegated call now
blocks until the save is taken.
This commit is contained in:
clawbot
2026-08-09 06:47:40 +00:00
committed by sneak
parent e1bf46b241
commit 3a01283358
13 changed files with 1176 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

123
ARCHITECTURE.md
View File

@@ -1484,9 +1484,11 @@ needed function pointers for — so it is also what the save format stores.
// Terminal is the physical device, behind an interface so tests run headless.
// The real game uses term.Tcell; tests use a scripted testTerm.
type Terminal interface {
Render(w *Window) // blit a window to the device
ReadChar() byte // event loop → C char codes (arrows→hjkl, ^C→quit)
Fini() // restore the device (curses endwin) on exit
Render(w *Window) // blit a window to the device
ReadChar() (byte, bool) // event loop → C char codes (arrows→hjkl, ^C→quit);
// ok is false when Interrupt woke the read
Interrupt() // wake a blocked ReadChar (signal goroutine only)
Fini() // restore the device (curses endwin) on exit
}
// Screen exposes the curses vocabulary the ported code speaks, over a Terminal.
@@ -1518,12 +1520,65 @@ SIGWINCH — SIGTSTP is not among the signals it takes, and is dropped (§9).
Signals are handled in `cmd/rogue/main.go`: one `os/signal` channel read by one
goroutine that reads exactly one signal, so a second signal can never call
`os.Exit` out from under an in-flight save. SIGHUP and SIGTERM `AutoSave` on the
`os.Exit` out from under an in-flight save. SIGHUP and SIGTERM autosave on the
way out (save.c `auto_save`); SIGINT and SIGQUIT restore the terminal and exit
without saving, matching C — where `auto_save` is reserved for HUP/TERM and
neither `leave()` nor `quit()` nor `endit()` writes a save file — and keeping a
deliberate interrupt from becoming a free checkpoint.
The signal goroutine does not write the save itself. It calls
`RogueGame.AutoSaveOnSignal`, which posts a request, wakes the input read
through `Terminal.Interrupt`, and waits for the game goroutine to take it or for
a deadline to run out; the encode happens on the game goroutine, the only one
that touches game state. It answers between turns (`command`), on waking from a
blocked `readchar`, and while parked in the `!` shell escape (`runShellEscape`)
— the three places it can sit for any length of time. Before issue #24 the
handler gob-encoded the live game tree from its own goroutine after removing the
save file, which raced every mutation the game was making and left a window with
no save file at all; `saveFile` now writes a temporary file in the save's own
directory and renames it over the target, so the previous save survives any
failure, including the deadline expiring with nothing written.
What that guarantees precisely, and what it does not: the encode runs on the one
goroutine that owns the state, so the snapshot is internally consistent and
always restorable. It is not guaranteed to be a between-commands snapshot. Only
one of the three service points gives that: the check at the top of `command`,
which runs after the previous command returned and before this turn's
`DoDaemons(Before)`/`DoFuses(Before)`. The other two are both reached from
inside a `command` call already under way. `readchar` is reached from prompts
raised part-way through a command (`--More--`, `askOverwrite`, `getStr`, the
direction and pack prompts), and the command has already mutated state by then —
`fight` sets `Count`/`Quiet` and runs `runTo` before any message, `revealXeroc`
writes `Disguise` before emitting one; the ordinary top-of-turn key read is
inside `command` too, after that turn's BEFORE daemons and `turnUpkeep`.
`runShellEscape` is no safer: `shell` is an ordinary command handler (`'!'` in
the dispatch table), reached through `executeCommand`, so a goroutine parked in
the shell escape has already run this turn's `DoDaemons(Before)`,
`DoFuses(Before)`, `turnUpkeep` and the last-command bookkeeping, and has not
yet run `DoDaemons(After)`, `DoFuses(After)` or `ringTurnEffects`. Restoring
re-enters `playit` at the top of `command` in either case, so the rest of that
command never runs — its AFTER daemons and fuses and its ring effects are lost —
and the restored game opens with a fresh BEFORE pass on top of the one already
in the snapshot: `rollwand`, a live BEFORE daemon once `swander` has fired,
ticks again, and any BEFORE fuse is decremented again. Not every consequence of
that pass is shared by both service points, though. `visuals` returns
immediately unless `g.After`, and `After` is part of the snapshot, so `DVisuals`
never re-ticks after a shell-escape save — `shell` sets `g.After = false` as its
first statement, before it parks — whereas after a `readchar` save it usually
does, because `turnUpkeep` sets `g.After = true` just before the top-of-turn
read; the exception is a handler that clears `After` before prompting, as
`identifyTrapCommand` does ahead of `promptDirection`. The result is a coherent
state one turn's worth of effects off, which is the price of being able to save
at all for a player whose line dropped mid-prompt or who is away in a shell.
The shell escape runs the shell on a helper goroutine so that the game goroutine
stays free to answer, but a panic out of `Terminal.ShellEscape` (which is how a
failed `Screen.Resume` is reported) is recovered there and re-raised on the game
goroutine. A panic reaching the top of a helper goroutine would kill the process
without running the main goroutine's `defer t.Fini()`, leaving the tty raw — the
failure issue #12 removed, on the one path where the terminal is already broken.
Re-raising keeps the invariant below true.
The handlers are installed immediately after `term.New()`, which is the call
that puts the tty in raw mode, and before the game exists — the saver is handed
over afterwards through `pendingSaver`. That ordering is what makes "every path
@@ -1611,26 +1666,26 @@ Tombstone/victory screens port verbatim from rip.c.
## 6. C construct → Go construct map
| C construct | Go translation |
| ---------------------------------------------------- | ----------------------------------------------------------------------------------------------- |
| global variable | `RogueGame` field (or Player/Level/subsystem field) |
| file-scope static | unexported field on `RogueGame` or subsystem struct |
| `THING` union | `Creature` / `Object` structs |
| linked lists (`l_next/l_prev`, attach/detach) | slices + `attach(&s, x)`-style helpers (prepend, remove by identity) |
| `coord*` aliasing (t_dest) | `*Coord` pointers into live structs (kept) |
| function pointers (daemons, options, nameit) | enum IDs → handler tables on `gameData` (daemons + effect dispatch); closures (options, nameit) |
| `when`/`otherwise`/`until` macros | `case`/`default`/`for !cond` |
| `goto` (retry loops in pack.c, move.c, passages.c) | labeled loops / restructured `for` |
| char-indexed tables (`monsters[type-'A']`) | same, `monsterTable[t.Type-'A']` |
| damage strings `"3x4/1x2"` | parsed once into a `DiceSpec` (`[]DiceRoll`) by `ParseDice` at table time |
| curses stdscr/hw windows | `Window` cell buffers over tcell |
| `mvinch` screen reads | `Window.Inch` from the buffer |
| signal handlers (SIGHUP autosave, SIGTSTP, SIGINT) | `os/signal` goroutine → channel checked in ReadChar; tcell handles TSTP/resize |
| `setjmp`-free exits (`my_exit`, `exit()` everywhere) | `myExit` restores the terminal and calls `os.Exit(0)`; one game run is one process |
| `vsprintf` message building | `fmt.Sprintf` |
| XOR-encrypted binary saves (state.c) | gob snapshot (§5.6) |
| DES wizard password | `ROGUE_WIZARD=1` env check |
| `md_*` platform shims | stdlib (`os`, `os/user`, `time`) or deleted |
| C construct | Go translation |
| ---------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- |
| global variable | `RogueGame` field (or Player/Level/subsystem field) |
| file-scope static | unexported field on `RogueGame` or subsystem struct |
| `THING` union | `Creature` / `Object` structs |
| linked lists (`l_next/l_prev`, attach/detach) | slices + `attach(&s, x)`-style helpers (prepend, remove by identity) |
| `coord*` aliasing (t_dest) | `*Coord` pointers into live structs (kept) |
| function pointers (daemons, options, nameit) | enum IDs → handler tables on `gameData` (daemons + effect dispatch); closures (options, nameit) |
| `when`/`otherwise`/`until` macros | `case`/`default`/`for !cond` |
| `goto` (retry loops in pack.c, move.c, passages.c) | labeled loops / restructured `for` |
| char-indexed tables (`monsters[type-'A']`) | same, `monsterTable[t.Type-'A']` |
| damage strings `"3x4/1x2"` | parsed once into a `DiceSpec` (`[]DiceRoll`) by `ParseDice` at table time |
| curses stdscr/hw windows | `Window` cell buffers over tcell |
| `mvinch` screen reads | `Window.Inch` from the buffer |
| signal handlers (SIGHUP autosave, SIGTSTP, SIGINT) | `os/signal` goroutine; the autosave is handed to the game goroutine and taken between turns or on waking a blocked `ReadChar`; tcell handles resize only |
| `setjmp`-free exits (`my_exit`, `exit()` everywhere) | `myExit` restores the terminal and calls `os.Exit(0)`; one game run is one process |
| `vsprintf` message building | `fmt.Sprintf` |
| XOR-encrypted binary saves (state.c) | gob snapshot (§5.6) |
| DES wizard password | `ROGUE_WIZARD=1` env check |
| `md_*` platform shims | stdlib (`os`, `os/user`, `time`) or deleted |
**Exit discipline** deserves a note: the C code calls `exit()` from deep inside
call chains (death, victory, save). One game run is one process, so the port
@@ -1752,13 +1807,19 @@ the signal goroutine while the game goroutine may be inside `Render` or
call from the signal goroutine is safe — it is a _logical_ race over the screen
state: the game goroutine can redraw into a screen the handler has just
suspended, or resume under a half-finished frame. Getting it right means
plumbing the signal through the input loop and handling it synchronously, a
design change well beyond a signal-safety fix. C's own wiring here is vestigial:
`tstp` is armed only by `md_tstpresume()`, which runs after a successful
`restore()`, so a freshly started C game never had a SIGTSTP handler either.
`term.Tcell.ShellEscape` (the `!` command) already covers getting to a shell and
back, doing the same suspend/resume dance synchronously on the game goroutine
where it is safe.
plumbing the signal through the input loop and handling it synchronously. Half
of that plumbing now exists — `Terminal.Interrupt` wakes a blocked `ReadChar` so
the game goroutine can act on a signal (§5.3, issue #24) — but the suspend and
the resume still have to be sequenced against the game's own drawing, which is
the part that remains a design change rather than a signal-safety fix. C's own
wiring here is vestigial: `tstp` is armed only by `md_tstpresume()`, which runs
after a successful `restore()`, so a freshly started C game never had a SIGTSTP
handler either. `term.Tcell.ShellEscape` (the `!` command) already covers
getting to a shell and back. Since issue #24 the shell itself runs on a helper
goroutine (`runShellEscape`), so a hangup arriving while the player is away in
the shell still rescues the game; the game goroutine waits there without
drawing, so the suspend/resume dance is as undisturbed as it was when the call
ran inline.
**SIGINT → `quit()`.** Under `md_onsignal_autosave` C routed SIGINT into the
interactive "really quit?" prompt. The port exits instead (after restoring the