1 Commits

Author SHA1 Message Date
clawbot
0dc4c70f18 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. readchar is reached from mid-command prompts (--More--,
askOverwrite, getStr, the direction and pack prompts) and the command has
already mutated state by then, so a save taken there freezes that command
half applied and restoring loses the rest of it.

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:07:23 +00:00
4 changed files with 33 additions and 76 deletions

View File

@@ -1541,28 +1541,16 @@ failure, including the deadline expiring with nothing written.
What that guarantees precisely, and what it does not: the encode runs on the one 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 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 always restorable. It is not guaranteed to be a between-commands snapshot.
one of the three service points gives that: the check at the top of `command`, `readchar` is also reached from prompts raised part-way through a command
which runs after the previous command returned and before this turn's (`--More--`, `askOverwrite`, `getStr`, the direction and pack prompts), and the
`DoDaemons(Before)`/`DoFuses(Before)`. The other two are both reached from command has already mutated state by then — `fight` sets `Count`/`Quiet` and
inside a `command` call already under way. `readchar` is reached from prompts runs `runTo` before any message, `revealXeroc` writes `Disguise` before emitting
raised part-way through a command (`--More--`, `askOverwrite`, `getStr`, the one. A save serviced at such a prompt freezes that command half applied;
direction and pack prompts), and the command has already mutated state by then — restoring re-enters `playit` at the top of `command`, so the rest of that
`fight` sets `Count`/`Quiet` and runs `runTo` before any message, `revealXeroc` command never runs. The result is a coherent state one turn's worth of effects
writes `Disguise` before emitting one; the ordinary top-of-turn key read is short, which is the price of being able to save at all for a player whose line
inside `command` too, after that turn's BEFORE daemons and `turnUpkeep`. dropped mid-prompt.
`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` ticks again, any BEFORE fuse is decremented again.
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

View File

@@ -27,18 +27,12 @@ encode game state from any goroutine but the game's.
Do not upgrade that into "the snapshot is always taken between commands" — it is Do not upgrade that into "the snapshot is always taken between commands" — it is
not. What is true is that the encode runs on the state-owning goroutine, so the not. What is true is that the encode runs on the state-owning goroutine, so the
snapshot is internally consistent and restorable. Only the check at the top of snapshot is internally consistent and restorable. `readchar` is reached from
`command` is a between-commands snapshot; the other two service points both sit
inside a `command` call already under way. `readchar` is reached from
mid-command prompts (`--More--`, `askOverwrite`, `getStr`, direction and pack mid-command prompts (`--More--`, `askOverwrite`, `getStr`, direction and pack
prompts) with the command's mutations already applied, and `runShellEscape` is prompts) and the command has already mutated state by then, so a save taken
reached from `shell`, an ordinary `'!'` command handler, with that turn's there freezes that command half applied and the player loses the rest of it on
`DoDaemons(Before)`/`DoFuses(Before)` already fired and its AFTER pass not yet. restore. That is acceptable and documented; the false stronger claim was caught
Restoring re-enters `playit` at the top of `command`, so either way the rest of in review of PR #26 and must not come back.
that command is lost and a fresh BEFORE pass runs on top of the one already in
the snapshot. That is acceptable and documented; two successive false claims —
first that `readchar` was safe, then that two of the three service points were
between-commands — were caught in review of PR #26, and neither may come back.
Related, and easy to reintroduce: work moved onto a helper goroutine must not be Related, and easy to reintroduce: work moved onto a helper goroutine must not be
allowed to panic there. A panic at the top of any goroutine kills the process allowed to panic there. A panic at the top of any goroutine kills the process

17
TODO.md
View File

@@ -89,17 +89,12 @@ wizard commands).
exactly the path where the terminal is already broken (issue #12's failure, exactly the path where the terminal is already broken (issue #12's failure,
reintroduced on a new path). `runShellEscape` recovers the helper's panic and reintroduced on a new path). `runShellEscape` recovers the helper's panic and
re-raises it on the game goroutine, pinned by re-raises it on the game goroutine, pinned by
`TestShellEscapePanicUnwindsTheGameGoroutine`. And the doc comment took two `TestShellEscapePanicUnwindsTheGameGoroutine`. And the first version of this
rounds to get right: the first version claimed in four places that nothing is work claimed in four places that nothing is half-mutated at the `readchar`
half-mutated at the `readchar` service point, and the revision that fixed that service point; that is false, since `readchar` is reached from mid-command
claimed two of the three service points were between-commands. Both are false. prompts. What is actually guaranteed is that the encode runs on the
Only the check at the top of `command` is between commands — `readchar` is state-owning goroutine, so the snapshot is internally consistent and
reached from mid-command prompts, and `runShellEscape` is reached from restorable, though it may freeze a command half applied. `Next Step`
`shell`, an ordinary `'!'` command handler dispatched inside `command`, with
that turn's `DoDaemons(Before)`/`DoFuses(Before)` already fired and its AFTER
pass and ring effects not yet. What is actually guaranteed is that the encode
runs on the state-owning goroutine, so the snapshot is internally consistent
and restorable, though it may freeze a command half applied. `Next Step`
deliberately not rotated: out-of-band issue work. deliberately not rotated: out-of-band issue work.
- 2026-08-09 Signal-time terminal restore (`sig-leave`, closes #12): the port - 2026-08-09 Signal-time terminal restore (`sig-leave`, closes #12): the port

View File

@@ -774,38 +774,18 @@ func (g *RogueGame) AutoSaveOnSignal(timeout time.Duration) bool {
// What is guaranteed, exactly: the encode runs on the one goroutine that // What is guaranteed, exactly: the encode runs on the one goroutine that
// owns the state, so the snapshot is internally consistent and always // owns the state, so the snapshot is internally consistent and always
// restorable. It is *not* guaranteed to be a between-commands snapshot. // 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 // Two of the three service points are, but readchar is reached from
// of command, which runs after the previous command returned and before // prompts raised part-way through a command — --More-- on the second
// this turn's DoDaemons(Before)/DoFuses(Before). The other two are both // message of a turn, askOverwrite, getStr, the direction and pack prompts
// reached from inside a command call already under way, and both cost // — and by then the command has already mutated state: fight sets
// the same on restore. // g.Count and g.Quiet and runs runTo before any message, revealXeroc
// // writes tp.Disguise before emitting one. A save serviced at such a
// readchar is reached from prompts raised part-way through a command — // prompt therefore freezes that command half applied; restoring re-enters
// --More-- on the second message of a turn, askOverwrite, getStr, the // playit at the top of command, so the rest of that command never runs
// direction and pack prompts — and by then the command has already // and the player loses its remaining effects. That is a coherent game
// mutated state: fight sets g.Count and g.Quiet and runs runTo before // state, one turn's worth of effects short — strictly better than the
// any message, revealXeroc writes tp.Disguise before emitting one. The // torn encode this replaced, and the cost of being able to save a player
// ordinary top-of-turn key read in readCommand is inside command too, // whose line dropped mid-prompt at all.
// after that turn's BEFORE daemons and turnUpkeep.
//
// runShellEscape is no safer. shell is an ordinary command handler ('!'
// in the tables.go 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.
//
// The cost, at both: restoring re-enters playit at the top of command,
// 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. That
// second BEFORE pass is not free: rollwand, a live Before daemon once
// 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
// 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
// a player whose line dropped mid-prompt, or who is away in a shell, at
// all.
func (g *RogueGame) serviceAutoSaveRequest() { func (g *RogueGame) serviceAutoSaveRequest() {
select { select {
case req := <-g.sigSave: case req := <-g.sigSave: