diff --git a/CHANGELOG.md b/CHANGELOG.md index 8be2d464..78d89035 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **The native window writes agent prompts in your prompt language.** The pull request and issue handoffs and a batch of review comments were always typed in English. +- **A theme change during a reconnect no longer crashes the native window.** + Switching the theme, or the system switching light and dark, while the + window reattached to a restarted lich could paint terminals the reconnect + had just freed and take the window down. - **Chinese, Japanese and Korean text no longer freezes the native window on Linux.** A session, a Review or a pull request full of CJK text could hang the window for seconds, long enough for "not responding", and a zoom diff --git a/native/cmd/lich-native/main.go b/native/cmd/lich-native/main.go index f0a416e8..e5c401d7 100644 --- a/native/cmd/lich-native/main.go +++ b/native/cmd/lich-native/main.go @@ -7,6 +7,7 @@ package main import ( "context" "crypto/rand" + "errors" "flag" "fmt" "image" @@ -728,7 +729,14 @@ func (u *window) content(gtx C, active sessionView) error { if err != nil { return err } - return u.terms.layout(gtx, t, u.terminalTakesKeys(gtx)) + err = u.terms.layout(gtx, t, u.terminalTakesKeys(gtx)) + // A reconnect frees every terminal off the frame goroutine; one shown + // since is replaced on the next frame, which the window must survive. + if errors.Is(err, gioterm.ErrClosed) { + gtx.Execute(op.InvalidateCmd{}) + return nil + } + return err } // terminalTakesKeys is whether the terminal on screen takes the keyboard back diff --git a/native/gioterm/links.go b/native/gioterm/links.go index 212d3ce7..1088d85e 100644 --- a/native/gioterm/links.go +++ b/native/gioterm/links.go @@ -66,6 +66,9 @@ type Link struct { func (v *Terminal) LinkAt(x, y float32) (l Link, ok bool) { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return Link{}, false + } col, row := v.cellAt(x, y) if uri := v.hyperlink(col, row); uri != "" { from, to := col, col diff --git a/native/gioterm/mouse.go b/native/gioterm/mouse.go index 926bcc62..5ef31f4c 100644 --- a/native/gioterm/mouse.go +++ b/native/gioterm/mouse.go @@ -161,6 +161,9 @@ func MouseButtonOf(b pointer.Buttons) MouseButton { func (v *Terminal) MouseTracking() bool { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return false + } return C.mouse_tracking(v.t) != C.GHOSTTY_MOUSE_TRACKING_NONE } @@ -171,6 +174,9 @@ func (v *Terminal) MouseTracking() bool { func (v *Terminal) EncodeMouse(action MouseAction, button MouseButton, mods key.Modifiers, x, y float32, anyPressed bool) []byte { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return nil + } if v.mouseEnc == nil { if C.ghostty_mouse_encoder_new(nil, &v.mouseEnc) != C.GHOSTTY_SUCCESS { return nil @@ -218,6 +224,9 @@ func (v *Terminal) SelectRelease(x, y float32) { func (v *Terminal) gesture(kind C.GhosttySelectionGestureEventType, x, y float32, at time.Duration) { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return + } if v.gest == nil && C.ghostty_selection_gesture_new(nil, &v.gest) != C.GHOSTTY_SUCCESS { return } @@ -243,6 +252,9 @@ func (v *Terminal) cellAt(x, y float32) (col, row int) { func (v *Terminal) ClearSelection() { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return + } C.ghostty_terminal_set(v.t, C.GHOSTTY_TERMINAL_OPT_SELECTION, nil) } @@ -251,6 +263,9 @@ func (v *Terminal) ClearSelection() { func (v *Terminal) SelectionText() string { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return "" + } var out *C.uint8_t var n C.size_t if C.selection_text(v.t, &out, &n) != C.GHOSTTY_SUCCESS { diff --git a/native/gioterm/paste.go b/native/gioterm/paste.go index 4576cefb..40caf0b8 100644 --- a/native/gioterm/paste.go +++ b/native/gioterm/paste.go @@ -33,6 +33,10 @@ const pasteFrame = 12 // with spaces either way. func (v *Terminal) Paste(text string) []byte { v.mu.Lock() + if v.closed { + v.mu.Unlock() + return nil + } bracketed := C.bracketed_paste(v.t) v.mu.Unlock() data := C.CBytes([]byte(text)) @@ -52,5 +56,8 @@ func (v *Terminal) Paste(text string) []byte { func (v *Terminal) AlternateScroll() bool { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return false + } return bool(C.alternate_scroll(v.t)) } diff --git a/native/gioterm/vt.go b/native/gioterm/vt.go index 85a84b33..e0a9d981 100644 --- a/native/gioterm/vt.go +++ b/native/gioterm/vt.go @@ -183,6 +183,7 @@ static GhosttyResult scroll_delta(GhosttyTerminal t, intptr_t delta) { import "C" import ( + "errors" "fmt" "image/color" "runtime/cgo" @@ -231,6 +232,7 @@ type Terminal struct { // runs inside Write with mu held; schemeSet is SetColors having set it. dark atomic.Bool schemeSet bool + closed bool t C.GhosttyTerminal rs C.GhosttyRenderState it C.GhosttyRenderStateRowIterator @@ -260,6 +262,9 @@ type Terminal struct { wheelRest float32 } +// ErrClosed is a call on a Terminal after Close. +var ErrClosed = errors.New("terminal closed") + func check(what string, r C.GhosttyResult) error { if r != C.GHOSTTY_SUCCESS { return fmt.Errorf("%s: ghostty result %d", what, int(r)) @@ -312,6 +317,10 @@ func (v *Terminal) Write(b []byte) { return } v.mu.Lock() + if v.closed { + v.mu.Unlock() + return + } C.ghostty_terminal_vt_write(v.t, (*C.uint8_t)(unsafe.Pointer(&b[0])), C.size_t(len(b))) v.mu.Unlock() } @@ -321,6 +330,9 @@ func (v *Terminal) Write(b []byte) { func (v *Terminal) Resize(cols, rows, cellW, cellH int) error { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return ErrClosed + } if err := check("terminal_resize", C.ghostty_terminal_resize(v.t, C.uint16_t(cols), C.uint16_t(rows), C.uint32_t(cellW), C.uint32_t(cellH))); err != nil { return err } @@ -333,6 +345,10 @@ func (v *Terminal) Resize(cols, rows, cellW, cellH int) error { // Scroll moves the viewport through scrollback, negative is up. func (v *Terminal) Scroll(delta int) { v.mu.Lock() + if v.closed { + v.mu.Unlock() + return + } C.scroll_delta(v.t, C.intptr_t(delta)) v.mu.Unlock() } @@ -394,6 +410,9 @@ type Colors struct { func (v *Terminal) SetColors(c Colors) error { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return ErrClosed + } if err := check("set colors", C.set_default_colors(v.t, cRGB(c.Foreground), cRGB(c.Background))); err != nil { return err } @@ -420,10 +439,15 @@ func cRGB(c color.NRGBA) C.GhosttyColorRgb { return C.GhosttyColorRgb{r: C.uint8_t(c.R), g: C.uint8_t(c.G), b: C.uint8_t(c.B)} } -// Close frees the terminal. It must not be used afterwards. +// Close frees the terminal. Calls after it do nothing: those returning an +// error return ErrClosed, the rest their zero value. Closing twice is a no-op. func (v *Terminal) Close() { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return + } + v.closed = true C.ghostty_selection_gesture_free(v.gest, v.t) C.ghostty_mouse_event_free(v.mouseEv) C.ghostty_mouse_encoder_free(v.mouseEnc) @@ -440,6 +464,9 @@ func (v *Terminal) Close() { func (v *Terminal) update() (snap, []uint8, error) { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return snap{}, nil, ErrClosed + } clear(v.dirty) s := C.snapshot(v.rs, v.t, v.it, v.cells, &v.raw[0], (*C.uint8_t)(&v.dirty[0]), C.uint16_t(v.cols), C.uint16_t(v.rows)) if s.dirty == -1 { @@ -515,6 +542,9 @@ const ( func (v *Terminal) encodeKey(key int, mods int, unshifted rune, text string) []byte { v.mu.Lock() defer v.mu.Unlock() + if v.closed { + return nil + } C.ghostty_key_encoder_setopt_from_terminal(v.enc, v.t) C.ghostty_key_event_set_action(v.ev, C.GHOSTTY_KEY_ACTION_PRESS) C.ghostty_key_event_set_key(v.ev, C.GhosttyKey(key)) diff --git a/native/gioterm/vt_test.go b/native/gioterm/vt_test.go index d9fc7178..b131ed75 100644 --- a/native/gioterm/vt_test.go +++ b/native/gioterm/vt_test.go @@ -1,6 +1,8 @@ package gioterm import ( + "errors" + "image/color" "testing" ) @@ -55,3 +57,36 @@ func TestEncodeSpecialKeysUnderKitty(t *testing.T) { }) } } + +// A window frees a terminal on one goroutine while another may still hold it: +// a reconnect closes every terminal as a theme switch paints them. +func TestClosedTerminalIgnoresCalls(t *testing.T) { + v, err := New(80, 24, testCellW, testCellH, func([]byte) {}) + if err != nil { + t.Fatal(err) + } + v.Close() + if err := v.SetColors(Colors{Background: color.NRGBA{A: 0xff}}); !errors.Is(err, ErrClosed) { + t.Fatalf("SetColors after Close: %v, want ErrClosed", err) + } + if err := v.Resize(100, 30, testCellW, testCellH); !errors.Is(err, ErrClosed) { + t.Fatalf("Resize after Close: %v, want ErrClosed", err) + } + if _, err := v.Update(); !errors.Is(err, ErrClosed) { + t.Fatalf("Update after Close: %v, want ErrClosed", err) + } + v.Write([]byte("text")) + v.Scroll(-1) + v.ClearSelection() + v.SelectPress(1, 1, 0) + if v.Paste("x") != nil || v.EncodeMouse(0, 0, 0, 1, 1, false) != nil || v.encodeKey(keyEnter, 0, 0, "") != nil { + t.Fatal("a closed terminal encoded input") + } + if v.MouseTracking() || v.AlternateScroll() || v.SelectionText() != "" { + t.Fatal("a closed terminal reported a mode") + } + if _, ok := v.LinkAt(1, 1); ok { + t.Fatal("a closed terminal found a link") + } + v.Close() +}