Compare commits
1 Commits
dfb34be1c4
...
f602ecdbbf
| Author | SHA1 | Date | |
|---|---|---|---|
| f602ecdbbf |
@@ -1513,8 +1513,18 @@ Ported drawing code keeps its structure: `mvaddch(y, x, ch)` →
|
||||
buffer, preserving the "screen is a data structure" idiom without touching the
|
||||
real terminal. `md_readchar`'s escape decoding is deleted; tcell's `EventKey`
|
||||
provides decoded keys and we translate to the byte codes `command()` already
|
||||
handles (KeyUp → 'k', etc.). SIGTSTP/resume and resize are handled by tcell;
|
||||
SIGHUP/SIGTERM → `autoSave` via `os/signal`.
|
||||
handles (KeyUp → 'k', etc.). Resize is handled by tcell, which registers only
|
||||
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
|
||||
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. Every path restores the
|
||||
terminal via `Terminal.Fini` before exiting, which is the whole point of C's
|
||||
`leave()`, "leave quickly but curteously".
|
||||
|
||||
### 5.4 Messaging
|
||||
|
||||
@@ -1715,6 +1725,39 @@ exit. Those are the steps referenced above (e.g. "step 5", "step 7").
|
||||
| tty dsusp/ltc character juggling | tcell owns the tty | none |
|
||||
| shell escape (`!`) setuid dance | no privileges to drop | plain `os/exec` shell |
|
||||
| curses window save in save file | screen is derivable | redraw on restore |
|
||||
| `tstp()` SIGTSTP suspend/resume | see below | `!` shell escape |
|
||||
| SIGINT → the interactive `quit()` prompt | see below | `Q`; SIGINT exits cleanly |
|
||||
| `auto_save` on SIGILL/TRAP/FPE/BUS/SEGV/SYS | see below | none |
|
||||
|
||||
The three signal rows warrant more than a table cell.
|
||||
|
||||
**SIGTSTP / `tstp()`.** Not handled, deliberately. Ctrl-Z cannot reach the game
|
||||
as a signal in the first place: tcell puts the tty in raw mode (`term.MakeRaw`
|
||||
clears `ISIG`, which is what makes `VSUSP` live), so Ctrl-Z arrives as key byte
|
||||
`0x1a`, exactly as it did in C, whose `setup()` calls curses `raw()` for the
|
||||
same effect. Only an explicit `kill -TSTP` can deliver it, which is not a player
|
||||
action. Handling it properly would mean calling `Screen.Suspend`/`Resume` from
|
||||
the signal goroutine while the game goroutine may be inside `Render` or
|
||||
`PollEvent` — a data race — so a correct port would have to plumb the signal
|
||||
through the input loop and handle 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.
|
||||
|
||||
**SIGINT → `quit()`.** Under `md_onsignal_autosave` C routed SIGINT into the
|
||||
interactive "really quit?" prompt. The port exits instead (after restoring the
|
||||
terminal): the handler runs on another goroutine, and re-entering the message
|
||||
and input machinery from there would race every screen and state access the main
|
||||
goroutine makes. The `Q` command reaches the same prompt from inside the turn
|
||||
loop, which is where the player actually quits.
|
||||
|
||||
**Fault signals.** `md_onsignal_autosave` also sent SIGILL, SIGTRAP, SIGFPE,
|
||||
SIGBUS, SIGSEGV and SIGSYS to `auto_save`. In Go these are runtime panics with
|
||||
their own diagnostics, and gob-encoding the state that just faulted would risk
|
||||
replacing a good save file with a corrupt one, so they are left alone. SIGHUP
|
||||
and SIGTERM still autosave.
|
||||
|
||||
## 10. Testing strategy
|
||||
|
||||
|
||||
42
TODO.md
42
TODO.md
@@ -34,6 +34,48 @@ wizard commands).
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-09 Signal-time terminal restore (`sig-leave`, closes #12): the port
|
||||
handled only SIGHUP and SIGTERM, so SIGINT and SIGQUIT killed the process with
|
||||
tcell still holding the tty, leaving the user at a shell with no echo. All
|
||||
four signals now go to one `os/signal` channel read by one goroutine in
|
||||
`cmd/rogue/main.go`, and every path calls `Terminal.Fini` before `os.Exit(0)`
|
||||
— C's `leave()`, "leave quickly but curteously". **The decision** (written
|
||||
into the `savesOnSignal` comment): SIGHUP/SIGTERM keep autosaving,
|
||||
SIGINT/SIGQUIT restore and exit **without** saving. C never saves on INT or
|
||||
QUIT anywhere — `leave()` is endwin-and-exit, `quit()` confirms/scores/exits,
|
||||
`endit()` goes through `fatal()`, and `save.c auto_save` is reserved for
|
||||
HUP/TERM — and the semantics agree: HUP/TERM are involuntary teardown worth
|
||||
rescuing a game from, while INT/QUIT are a deliberate "stop now" that must not
|
||||
become a one-keystroke checkpoint against a save discipline built to be
|
||||
anti-save-scum. It is also the safe choice: `AutoSave` gob-encodes live state
|
||||
that the main goroutine is still mutating, after removing the old file, so on
|
||||
the signals with nothing to rescue the port takes the option with no
|
||||
corruption window. The single-reader design closes the window the issue warned
|
||||
about: a second signal arriving mid-save stays unread in the buffer instead of
|
||||
exiting out from under the writer (`TestLeaveOnSignalIgnoresLaterSignals`
|
||||
reproduces exactly that interleaving). New `cmd/rogue/main_test.go` covers the
|
||||
ordering for each signal, the save/no-save split against `savesOnSignal`, the
|
||||
mid-save-second-signal case, and real SIGINT/SIGQUIT/SIGHUP/SIGTERM delivered
|
||||
to the test process through the same `notifySignals` wiring the game uses; the
|
||||
tty leaving raw mode is the one step not checkable headlessly (it needs a
|
||||
controlling terminal), and `term.Tcell.Fini` is a direct pass-through to
|
||||
tcell's `Screen.Fini` that `myExit` already depends on. Two premises in the
|
||||
issue turned out to be wrong and are recorded in ARCHITECTURE.md: `leave()` is
|
||||
not installed on SIGINT/SIGQUIT during play (the wiring is in `mdport.c`, the
|
||||
shipped build calls `md_onsignal_default()` and installs nothing, and
|
||||
`leave()` appears only in the endgame paths of `rip.c`/`main.c`), and Ctrl-C
|
||||
never generated SIGINT here anyway, since tcell's raw mode clears `ISIG` and
|
||||
the key arrives as byte `0x03` — as it did in C, whose `setup()` calls curses
|
||||
`raw()`. The real exposure is `kill -INT`/`kill -QUIT` and the window before
|
||||
`term.New()`. ARCHITECTURE.md §9 gained rows for SIGTSTP/`tstp()` (dropped:
|
||||
raw mode means Ctrl-Z cannot reach us, a suspend from the signal goroutine
|
||||
would race the drawing goroutine, and C armed `tstp` only after a `restore()`;
|
||||
the `!` shell escape covers the need), for SIGINT not routing to the
|
||||
interactive `quit()` prompt, and for `auto_save` on the fault signals; §5.3's
|
||||
claim that tcell handles SIGTSTP was false — tcell registers only SIGWINCH —
|
||||
and is corrected. `Next Step` deliberately not rotated: out-of-band issue
|
||||
work.
|
||||
|
||||
- 2026-08-09 Wizard-create bounds fix (`fix/wizard-which-bounds`, closes #10):
|
||||
`createObj` stored the raw `0-f` nibble as `Object.Which` with no bounds
|
||||
check, so wizard mode -> `C` -> `/` -> `f` produced a wand numbered 15 against
|
||||
|
||||
@@ -69,7 +69,7 @@ func run() int {
|
||||
return 0
|
||||
}
|
||||
|
||||
installAutosave(g, t)
|
||||
installSignalHandlers(g, t)
|
||||
|
||||
g.Run() // does not return: the game ends by exiting the process
|
||||
|
||||
@@ -101,19 +101,102 @@ func loadParams() game.Params {
|
||||
}
|
||||
}
|
||||
|
||||
// installAutosave saves the game and exits on SIGHUP/SIGTERM (save.c
|
||||
// auto_save).
|
||||
func installAutosave(g *game.RogueGame, t *term.Tcell) {
|
||||
// saver is the autosave half of *game.RogueGame that the signal handler
|
||||
// needs; an interface so the handler is testable headlessly.
|
||||
type saver interface {
|
||||
// AutoSave writes the game to its save file, best effort (save.c
|
||||
// auto_save).
|
||||
AutoSave()
|
||||
}
|
||||
|
||||
// finisher is the terminal-restoring half of game.Terminal that the
|
||||
// signal handler needs (curses endwin).
|
||||
type finisher interface {
|
||||
// Fini restores the terminal to its pre-game state.
|
||||
Fini()
|
||||
}
|
||||
|
||||
// handledSignals returns the signals the game leaves on. They split into
|
||||
// two groups with deliberately different save behavior; see
|
||||
// savesOnSignal.
|
||||
func handledSignals() []os.Signal {
|
||||
return []os.Signal{
|
||||
syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT, syscall.SIGQUIT,
|
||||
}
|
||||
}
|
||||
|
||||
// savesOnSignal reports whether the game should autosave on its way out
|
||||
// for this signal.
|
||||
//
|
||||
// THE DECISION (issue #12): SIGHUP and SIGTERM save; SIGINT and SIGQUIT
|
||||
// restore the terminal and exit WITHOUT saving. This is deliberate, not
|
||||
// an oversight, on three grounds.
|
||||
//
|
||||
// C: no path in the C game saves on INT or QUIT. The shipped build
|
||||
// installs no handler at all during play (mach_dep.c setup calls
|
||||
// md_onsignal_default), and the only INT handler it ever installs is
|
||||
// rip.c/main.c's leave() in the endgame — endwin and exit, explicitly
|
||||
// discarding pending output. The build that does wire INT during play
|
||||
// (md_onsignal_autosave, mdport.c, compiled only under DUMP) sends it to
|
||||
// quit(), which confirms, scores, and exits, again without saving, and
|
||||
// sends QUIT to endit() -> fatal() -> endwin + exit. save.c auto_save is
|
||||
// reserved for HUP/TERM. Saving on HUP/TERM but not on INT/QUIT is
|
||||
// therefore exactly C's split.
|
||||
//
|
||||
// Semantics: HUP and TERM mean involuntary teardown — the line dropped
|
||||
// or the machine is going down — so rescuing the game is right. INT and
|
||||
// QUIT are the player deliberately saying "stop now". Rogue scores a
|
||||
// deliberate quit, and its save discipline is anti-save-scum by design
|
||||
// (restoring consumes the file), so making Ctrl-C a free checkpoint
|
||||
// would turn it into a one-keystroke undo for a bad turn: a gameplay
|
||||
// change, not a robustness fix.
|
||||
//
|
||||
// Safety: this runs on a goroutine while the main goroutine is mid-turn
|
||||
// mutating game state, and AutoSave removes the save file before
|
||||
// gob-encoding that live state. On HUP/TERM that risk is accepted
|
||||
// because the process is about to die regardless and a best-effort save
|
||||
// beats none. On INT/QUIT there is nothing to rescue, so the right
|
||||
// choice is the one with no corruption window at all.
|
||||
func savesOnSignal(sig os.Signal) bool {
|
||||
return sig == syscall.SIGHUP || sig == syscall.SIGTERM
|
||||
}
|
||||
|
||||
// installSignalHandlers arranges for the game to leave the terminal
|
||||
// usable when it is signalled: C's leave(), "leave quickly but
|
||||
// curteously" (main.c), extended with save.c auto_save on the two
|
||||
// signals that warrant it.
|
||||
func installSignalHandlers(g saver, t finisher) {
|
||||
go leaveOnSignal(notifySignals(), g, t, os.Exit)
|
||||
}
|
||||
|
||||
// notifySignals subscribes to the handled signals and returns the
|
||||
// channel they arrive on. Split out from installSignalHandlers so tests
|
||||
// can drive leaveOnSignal with real signal delivery.
|
||||
func notifySignals() chan os.Signal {
|
||||
// Buffered so signal delivery never blocks, and deliberately never
|
||||
// drained past the first signal: see leaveOnSignal.
|
||||
sig := make(chan os.Signal, 1)
|
||||
|
||||
signal.Notify(sig, syscall.SIGHUP, syscall.SIGTERM)
|
||||
signal.Notify(sig, handledSignals()...)
|
||||
|
||||
go func() {
|
||||
<-sig
|
||||
return sig
|
||||
}
|
||||
|
||||
// leaveOnSignal waits for one signal and takes the game out.
|
||||
//
|
||||
// Exactly one goroutine reads exactly one signal, which is what makes
|
||||
// the exit safe: a second signal (a SIGINT landing while a SIGHUP's
|
||||
// AutoSave is still writing, say) stays in the buffer unread and can
|
||||
// never call exit out from under an in-flight save. The order within is
|
||||
// the same one myExit uses (game/rip.go): save if this signal saves,
|
||||
// then restore the terminal, then exit.
|
||||
func leaveOnSignal(sig <-chan os.Signal, g saver, t finisher, exit func(int)) {
|
||||
if savesOnSignal(<-sig) {
|
||||
g.AutoSave()
|
||||
}
|
||||
|
||||
t.Fini()
|
||||
os.Exit(0)
|
||||
}()
|
||||
exit(0)
|
||||
}
|
||||
|
||||
// chooseSeed picks the dungeon number: SEED for reproducible dungeons
|
||||
|
||||
222
cmd/rogue/main_test.go
Normal file
222
cmd/rogue/main_test.go
Normal file
@@ -0,0 +1,222 @@
|
||||
package main
|
||||
|
||||
// White-box tests for the signal plumbing. Unlike the game package's test
|
||||
// files this one carries no //nolint:testpackage directive: testpackage
|
||||
// exempts package main, so nolintlint rejects the directive as unused.
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/signal"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
"syscall"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// signalRecorder stands in for the game and the terminal in the signal
|
||||
// handler, recording the order of the steps the handler takes. The mutex
|
||||
// matters: the handler runs on its own goroutine, so an unguarded slice
|
||||
// would be a data race under -race, which is exactly what these tests
|
||||
// are meant to rule out.
|
||||
type signalRecorder struct {
|
||||
mu sync.Mutex
|
||||
steps []string
|
||||
code int
|
||||
done chan struct{}
|
||||
}
|
||||
|
||||
func newSignalRecorder() *signalRecorder {
|
||||
return &signalRecorder{done: make(chan struct{})}
|
||||
}
|
||||
|
||||
// AutoSave records a save attempt (the saver half).
|
||||
func (r *signalRecorder) AutoSave() {
|
||||
r.record("save")
|
||||
}
|
||||
|
||||
// Fini records a terminal restore (the finisher half).
|
||||
func (r *signalRecorder) Fini() {
|
||||
r.record("fini")
|
||||
}
|
||||
|
||||
// exit records the process exit that ends the handler and releases any
|
||||
// waiter. It stands in for os.Exit, which cannot be called in a test.
|
||||
func (r *signalRecorder) exit(code int) {
|
||||
r.mu.Lock()
|
||||
r.code = code
|
||||
r.steps = append(r.steps, "exit")
|
||||
r.mu.Unlock()
|
||||
|
||||
close(r.done)
|
||||
}
|
||||
|
||||
// record appends one step.
|
||||
func (r *signalRecorder) record(step string) {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
r.steps = append(r.steps, step)
|
||||
}
|
||||
|
||||
// taken returns the recorded steps and the exit code.
|
||||
func (r *signalRecorder) taken() ([]string, int) {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
return slices.Clone(r.steps), r.code
|
||||
}
|
||||
|
||||
// TestLeaveOnSignalRestoresTerminalBeforeExit is the core of issue #12:
|
||||
// whatever the signal, the terminal is restored before the process ends,
|
||||
// so the player is never dropped into a shell with the tty still in raw
|
||||
// mode.
|
||||
func TestLeaveOnSignalRestoresTerminalBeforeExit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, sig := range handledSignals() {
|
||||
rec := newSignalRecorder()
|
||||
|
||||
ch := make(chan os.Signal, 1)
|
||||
ch <- sig
|
||||
|
||||
leaveOnSignal(ch, rec, rec, rec.exit)
|
||||
|
||||
steps, code := rec.taken()
|
||||
|
||||
fini := slices.Index(steps, "fini")
|
||||
exit := slices.Index(steps, "exit")
|
||||
|
||||
if fini < 0 || exit < 0 || fini > exit {
|
||||
t.Errorf("%v: want the terminal restored before exit, got %v",
|
||||
sig, steps)
|
||||
}
|
||||
|
||||
if code != 0 {
|
||||
t.Errorf("%v: exit code = %d, want 0", sig, code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestLeaveOnSignalSaveSplit pins the decision recorded on savesOnSignal:
|
||||
// SIGHUP/SIGTERM (involuntary teardown) save on the way out, SIGINT and
|
||||
// SIGQUIT (a deliberate "stop now" from the player) do not, matching C,
|
||||
// where auto_save is reserved for HUP/TERM and neither leave() nor quit()
|
||||
// nor endit() writes a save file.
|
||||
func TestLeaveOnSignalSaveSplit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
want := map[os.Signal][]string{
|
||||
syscall.SIGHUP: {"save", "fini", "exit"},
|
||||
syscall.SIGTERM: {"save", "fini", "exit"},
|
||||
syscall.SIGINT: {"fini", "exit"},
|
||||
syscall.SIGQUIT: {"fini", "exit"},
|
||||
}
|
||||
|
||||
for _, sig := range handledSignals() {
|
||||
rec := newSignalRecorder()
|
||||
|
||||
ch := make(chan os.Signal, 1)
|
||||
ch <- sig
|
||||
|
||||
leaveOnSignal(ch, rec, rec, rec.exit)
|
||||
|
||||
steps, _ := rec.taken()
|
||||
if !slices.Equal(steps, want[sig]) {
|
||||
t.Errorf("%v: steps = %v, want %v", sig, steps, want[sig])
|
||||
}
|
||||
|
||||
if saved := slices.Contains(steps, "save"); saved != savesOnSignal(sig) {
|
||||
t.Errorf("%v: saved = %v, savesOnSignal = %v",
|
||||
sig, saved, savesOnSignal(sig))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestLeaveOnSignalIgnoresLaterSignals covers the ordering guarantee in
|
||||
// leaveOnSignal's comment: only the first signal is read, so a second one
|
||||
// arriving mid-save cannot exit out from under the save and truncate the
|
||||
// player's file. The saver here blocks until a second signal has been
|
||||
// queued, reproducing that window.
|
||||
func TestLeaveOnSignalIgnoresLaterSignals(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := newSignalRecorder()
|
||||
|
||||
ch := make(chan os.Signal, 2)
|
||||
ch <- syscall.SIGHUP
|
||||
|
||||
blocker := &blockingSaver{rec: rec, queue: ch, extra: syscall.SIGINT}
|
||||
|
||||
leaveOnSignal(ch, blocker, rec, rec.exit)
|
||||
|
||||
steps, _ := rec.taken()
|
||||
if !slices.Equal(steps, []string{"save", "fini", "exit"}) {
|
||||
t.Errorf("steps = %v, want one save, one fini, one exit", steps)
|
||||
}
|
||||
|
||||
if len(ch) != 1 {
|
||||
t.Errorf("queued signals left unread = %d, want 1", len(ch))
|
||||
}
|
||||
}
|
||||
|
||||
// blockingSaver queues another signal while the save is in flight, the
|
||||
// race window leaveOnSignal is built to close.
|
||||
type blockingSaver struct {
|
||||
rec *signalRecorder
|
||||
queue chan os.Signal
|
||||
extra os.Signal
|
||||
}
|
||||
|
||||
// AutoSave delivers the extra signal mid-save, then records the save.
|
||||
func (b *blockingSaver) AutoSave() {
|
||||
b.queue <- b.extra
|
||||
|
||||
b.rec.AutoSave()
|
||||
}
|
||||
|
||||
// TestLeaveOnRealSignal is the deepest headless check available: it
|
||||
// delivers real SIGINT/SIGQUIT/SIGHUP/SIGTERM to this process through
|
||||
// os/signal, exactly as notifySignals wires them in the game, and
|
||||
// verifies each one reaches the handler and restores the terminal before
|
||||
// exiting.
|
||||
//
|
||||
// What cannot be checked here is the tty itself coming back out of raw
|
||||
// mode: that needs a controlling terminal and a live tcell screen, which
|
||||
// a headless test run does not have. This test covers everything up to
|
||||
// the Terminal.Fini call; term.Tcell.Fini is a direct pass-through to
|
||||
// tcell's Screen.Fini, which is the same call myExit already relies on.
|
||||
func TestLeaveOnRealSignal(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ch := notifySignals()
|
||||
defer signal.Stop(ch)
|
||||
|
||||
for _, sig := range handledSignals() {
|
||||
rec := newSignalRecorder()
|
||||
|
||||
go leaveOnSignal(ch, rec, rec, rec.exit)
|
||||
|
||||
unix, ok := sig.(syscall.Signal)
|
||||
if !ok {
|
||||
t.Fatalf("%v is not a unix signal", sig)
|
||||
}
|
||||
|
||||
err := syscall.Kill(os.Getpid(), unix)
|
||||
if err != nil {
|
||||
t.Fatalf("kill(%v): %v", sig, err)
|
||||
}
|
||||
|
||||
<-rec.done
|
||||
|
||||
steps, code := rec.taken()
|
||||
if !strings.HasSuffix(strings.Join(steps, ","), "fini,exit") {
|
||||
t.Errorf("%v: steps = %v, want the terminal restored before exit",
|
||||
sig, steps)
|
||||
}
|
||||
|
||||
if code != 0 {
|
||||
t.Errorf("%v: exit code = %d, want 0", sig, code)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user