From b099dbe6bf985563ac1880aaa4d0e14fc347b3dc Mon Sep 17 00:00:00 2001 From: omartelo Date: Sat, 17 Oct 2026 14:58:50 -0300 Subject: [PATCH] fix(native): ignore terminal calls after Close A reconnect frees every terminal from its own goroutine while the frame goroutine may still paint one: setColors since #780 snapshots the views and paints them after releasing the lock, and a frame can hold the shown terminal across the swap. Those calls ran on freed libghostty memory. Close now marks the Terminal closed and every method after it does nothing, returning ErrClosed where it returns an error. The frame that meets ErrClosed is redrawn instead of ending the event loop. Refs #777 --- CHANGELOG.md | 4 ++++ native/cmd/lich-native/main.go | 10 +++++++++- native/gioterm/links.go | 3 +++ native/gioterm/mouse.go | 15 +++++++++++++++ native/gioterm/paste.go | 7 +++++++ native/gioterm/vt.go | 32 ++++++++++++++++++++++++++++++- native/gioterm/vt_test.go | 35 ++++++++++++++++++++++++++++++++++ 7 files changed, 104 insertions(+), 2 deletions(-) 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() +}