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.
This commit is contained in:
@@ -8,6 +8,13 @@ package game
|
||||
func (g *RogueGame) command() {
|
||||
p := &g.Player
|
||||
|
||||
// Between turns is the one point in the loop where the game state is
|
||||
// whole, so it is where a signal-triggered autosave is answered when
|
||||
// the game goroutine is busy rather than waiting for a key (issue
|
||||
// #24). The other service points are readchar (io.c) and
|
||||
// runShellEscape, covering the two ways this goroutine can be parked.
|
||||
g.serviceAutoSaveRequest()
|
||||
|
||||
ntimes := 1 // number of player moves
|
||||
if p.On(Hasted) {
|
||||
ntimes++
|
||||
@@ -890,7 +897,7 @@ func (g *RogueGame) shell() {
|
||||
if se, ok := g.scr.term.(interface{ ShellEscape() }); ok {
|
||||
g.InShell = true
|
||||
|
||||
se.ShellEscape()
|
||||
g.runShellEscape(se)
|
||||
|
||||
g.InShell = false
|
||||
g.refresh()
|
||||
@@ -898,3 +905,58 @@ func (g *RogueGame) shell() {
|
||||
g.msg("shell escape is not available")
|
||||
}
|
||||
}
|
||||
|
||||
// runShellEscape runs the shell and returns when it exits, answering
|
||||
// signal-triggered autosave requests in the meantime (issue #24).
|
||||
//
|
||||
// The shell blocks for as long as the player is away — minutes, or until
|
||||
// they forget — and a line dropping while they are in it is exactly the
|
||||
// case SIGHUP autosave exists for, so this goroutine cannot simply sit
|
||||
// inside the call. The shell runs on a helper goroutine and the game
|
||||
// goroutine waits here, still the only one that ever encodes game state.
|
||||
// It draws nothing while it waits, so the suspend/resume dance is as
|
||||
// undisturbed as it was when it ran inline (ARCHITECTURE.md section 9).
|
||||
//
|
||||
// A panic out of ShellEscape must not be allowed to unwind on the helper
|
||||
// goroutine. term.Tcell.ShellEscape panics when Screen.Resume fails, and
|
||||
// a panic reaching the top of any goroutine kills the process without
|
||||
// running any *other* goroutine's deferred calls — which is where
|
||||
// cmd/rogue/main.go's `defer t.Fini()` lives. Running the shell off the
|
||||
// game goroutine would therefore have left the tty raw on exactly the
|
||||
// path where the terminal is already broken, reintroducing issue #12 on a
|
||||
// path this change created. So the helper recovers, and the value is
|
||||
// re-raised below on the game goroutine, whose stack does have Fini in
|
||||
// it. The recover deferral is registered after `defer close(done)` and so
|
||||
// runs before it, which is what publishes panicVal to the reader.
|
||||
func (g *RogueGame) runShellEscape(se interface{ ShellEscape() }) {
|
||||
done := make(chan struct{})
|
||||
|
||||
var panicVal any
|
||||
|
||||
go func() {
|
||||
defer close(done)
|
||||
|
||||
defer func() {
|
||||
panicVal = recover()
|
||||
}()
|
||||
|
||||
se.ShellEscape()
|
||||
}()
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-done:
|
||||
if panicVal != nil {
|
||||
// Re-raised here so the unwind passes through the game
|
||||
// goroutine's deferred Fini. shell()'s InShell reset and
|
||||
// refresh are skipped deliberately: there is no screen
|
||||
// left to draw into.
|
||||
panic(panicVal)
|
||||
}
|
||||
|
||||
return
|
||||
case req := <-g.sigSave:
|
||||
g.runAutoSaveRequest(req)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user