1 Commits

Author SHA1 Message Date
clawbot
3bc2e09e24 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:31:46 +00:00
4 changed files with 76 additions and 33 deletions

View File

@@ -1541,16 +1541,28 @@ 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.
`readchar` is also 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. A save serviced at such a prompt freezes that command half applied;
restoring re-enters `playit` at the top of `command`, so the rest of that
command never runs. The result is a coherent state one turn's worth of effects
short, which is the price of being able to save at all for a player whose line
dropped mid-prompt.
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` 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
stays free to answer, but a panic out of `Terminal.ShellEscape` (which is how a

View File

@@ -27,12 +27,18 @@ encode game state from any goroutine but the game's.
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
snapshot is internally consistent and restorable. `readchar` is reached from
snapshot is internally consistent and restorable. Only the check at the top of
`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
prompts) and the command has already mutated state by then, so a save taken
there freezes that command half applied and the player loses the rest of it on
restore. That is acceptable and documented; the false stronger claim was caught
in review of PR #26 and must not come back.
prompts) with the command's mutations already applied, and `runShellEscape` is
reached from `shell`, an ordinary `'!'` command handler, with that turn's
`DoDaemons(Before)`/`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 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
allowed to panic there. A panic at the top of any goroutine kills the process

17
TODO.md
View File

@@ -89,12 +89,17 @@ wizard commands).
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
re-raises it on the game goroutine, pinned by
`TestShellEscapePanicUnwindsTheGameGoroutine`. And the first version of this
work claimed in four places that nothing is half-mutated at the `readchar`
service point; that is false, since `readchar` is reached from mid-command
prompts. 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`
`TestShellEscapePanicUnwindsTheGameGoroutine`. And the doc comment took two
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
claimed two of the three service points were between-commands. Both are false.
Only the check at the top of `command` is between commands — `readchar` is
reached from mid-command prompts, and `runShellEscape` is reached from
`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.
- 2026-08-09 Signal-time terminal restore (`sig-leave`, closes #12): the port

View File

@@ -774,18 +774,38 @@ func (g *RogueGame) AutoSaveOnSignal(timeout time.Duration) bool {
// What is guaranteed, exactly: 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.
// Two of the three service points are, but readchar is reached from
// prompts raised part-way through a command — --More-- on the second
// message of a turn, askOverwrite, getStr, the direction and pack prompts
// — and by then the command has already mutated state: fight sets
// g.Count and g.Quiet and runs runTo before any message, revealXeroc
// writes tp.Disguise before emitting one. A save serviced at such a
// prompt therefore freezes that command half applied; restoring re-enters
// playit at the top of command, so the rest of that command never runs
// and the player loses its remaining effects. That is a coherent game
// state, one turn's worth of effects short — strictly better than the
// torn encode this replaced, and the cost of being able to save a player
// whose line dropped mid-prompt at all.
// 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, and both cost
// the same on restore.
//
// readchar is reached from prompts raised part-way through a command —
// --More-- on the second message of a turn, askOverwrite, getStr, the
// direction and pack prompts — and by then the command has already
// mutated state: fight sets g.Count and g.Quiet and runs runTo before
// any message, revealXeroc writes tp.Disguise before emitting one. The
// ordinary top-of-turn key read in readCommand is inside command too,
// 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() {
select {
case req := <-g.sigSave: