1 Commits

Author SHA1 Message Date
clawbot
3a01283358 fix: take the signal-time autosave on the game goroutine (closes #24)
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.
2026-08-09 07:50:23 +00:00
4 changed files with 50 additions and 30 deletions

View File

@@ -1559,10 +1559,17 @@ 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 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 — 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 and the restored game opens with a fresh BEFORE pass on top of the one already
in the snapshot: `rollwand` ticks again, any BEFORE fuse is decremented again. in the snapshot: `rollwand`, a live BEFORE daemon once `swander` has fired,
The result is a coherent state one turn's worth of effects off, which is the ticks again, and any BEFORE fuse is decremented again. Not every consequence of
price of being able to save at all for a player whose line dropped mid-prompt or that pass is shared by both service points, though. `visuals` returns
who is away in a shell. 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 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 stays free to answer, but a panic out of `Terminal.ShellEscape` (which is how a

38
TODO.md
View File

@@ -70,25 +70,25 @@ wizard commands).
the save, which still reads the old file whole after it. Each was the save, which still reads the old file whole after it. Each was
mutation-proved: reverting `AutoSaveOnSignal` to encode on the calling mutation-proved: reverting `AutoSaveOnSignal` to encode on the calling
goroutine (the pre-fix behavior) makes the turn-loop test fail under `-race` goroutine (the pre-fix behavior) makes the turn-loop test fail under `-race`
with 113 reports, and removing each of the three service points fails exactly with over a hundred reports, and removing each of the three service points
the test for that park with its own message. `pendingSaver` now reads the game fails exactly the test for that park with its own message. `pendingSaver` now
out from under its mutex instead of delegating with it held, because the reads the game out from under its mutex instead of delegating with it held,
delegated call blocks until the save is taken — the PR #23 review's N3 note, because the delegated call blocks until the save is taken — the PR #23
load-bearing rather than hypothetical, and pinned by a test. The review's N3 note, load-bearing rather than hypothetical, and pinned by a test.
SIGINT/SIGQUIT no-save decision and the single-signal-read ordering guarantee The SIGINT/SIGQUIT no-save decision and the single-signal-read ordering
are untouched; `savesOnSignal`'s third ground ("safety") is rewritten, since guarantee are untouched; `savesOnSignal`'s third ground ("safety") is
the corruption window it weighed no longer exists. `MEMORY.md` stops listing rewritten, since the corruption window it weighed no longer exists.
signal-time autosave among the deliberate `_ =` discards and states the new `MEMORY.md` stops listing signal-time autosave among the deliberate `_ =`
discipline; `ARCHITECTURE.md` §5.3, the `Terminal` sketch, the C-to-Go mapping discards and states the new discipline; `ARCHITECTURE.md` §5.3, the `Terminal`
row and §9's SIGTSTP paragraph are corrected to match. Two things review sketch, the C-to-Go mapping row and §9's SIGTSTP paragraph are corrected to
caught and this entry records so they are not undone: moving the shell onto a match. Two things review caught and this entry records so they are not undone:
helper goroutine also moved `term.Tcell.ShellEscape`'s `panic` on a failed moving the shell onto a helper goroutine also moved `term.Tcell.ShellEscape`'s
`Screen.Resume` there, and a panic at the top of any goroutine kills the `panic` on a failed `Screen.Resume` there, and a panic at the top of any
process without running the deferred calls of the others — including goroutine kills the process without running the deferred calls of the others —
`cmd/rogue/main.go`'s `defer t.Fini()`, so the tty would have been left raw on including `cmd/rogue/main.go`'s `defer t.Fini()`, so the tty would have been
exactly the path where the terminal is already broken (issue #12's failure, left raw on exactly the path where the terminal is already broken (issue #12's
reintroduced on a new path). `runShellEscape` recovers the helper's panic and failure, reintroduced on a new path). `runShellEscape` recovers the helper's
re-raises it on the game goroutine, pinned by panic and re-raises it on the game goroutine, pinned by
`TestShellEscapePanicUnwindsTheGameGoroutine`. And the doc comment took two `TestShellEscapePanicUnwindsTheGameGoroutine`. And the doc comment took two
rounds to get right: the first version claimed in four places that nothing is rounds to get right: the first version claimed in four places that nothing is
half-mutated at the `readchar` service point, and the revision that fixed that half-mutated at the `readchar` service point, and the revision that fixed that

View File

@@ -174,12 +174,14 @@ func stepOk(ch byte) bool {
// save runs here, on the game goroutine, before reading again. // save runs here, on the game goroutine, before reading again.
// //
// What that buys is a snapshot taken by the goroutine that owns the // What that buys is a snapshot taken by the goroutine that owns the
// state, so it is internally consistent and restorable. It is not // state, so it is internally consistent and restorable. It is never a
// necessarily a between-commands snapshot: readchar is also reached from // between-commands snapshot: readchar is reached from readCommand at the
// prompts raised part-way through a command — --More--, askOverwrite, // top of a turn that has already run its BEFORE daemons and turnUpkeep,
// getStr, the direction and pack prompts — and mutation has already // and from prompts raised part-way through a command — --More--,
// happened by then. See serviceAutoSaveRequest (save.go) for what that // askOverwrite, getStr, the direction and pack prompts — by which point
// costs the player. // the command has mutated state as well. See serviceAutoSaveRequest
// (save.go) for the full statement of what the handoff guarantees and
// what it costs the player.
func (g *RogueGame) readchar() byte { func (g *RogueGame) readchar() byte {
for { for {
ch, ok := g.scr.term.ReadChar() ch, ok := g.scr.term.ReadChar()

View File

@@ -801,7 +801,18 @@ func (g *RogueGame) AutoSaveOnSignal(timeout time.Duration) bool {
// fresh BEFORE pass on top of the one already in the snapshot. That // fresh BEFORE pass on top of the one already in the snapshot. That
// second BEFORE pass is not free: rollwand, a live Before daemon once // second BEFORE pass is not free: rollwand, a live Before daemon once
// swander has fired, ticks again and draws from the RNG every fourth // swander has fired, ticks again and draws from the RNG every fourth
// tick, and any Before fuse is decremented again. The result is still a // tick, and any Before fuse is decremented again.
//
// Not every consequence of that pass is shared by both, 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. After a
// readchar save it usually does re-tick, 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 still a
// coherent game state, one turn's worth of effects off — strictly better // coherent game state, one turn's worth of effects off — strictly better
// than the torn encode this replaced, and the cost of being able to save // than the torn encode this replaced, and the cost of being able to save
// a player whose line dropped mid-prompt, or who is away in a shell, at // a player whose line dropped mid-prompt, or who is away in a shell, at