From ec577f1afa7ff0e9933f3e511ed65d941ab039a5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 12 Aug 2026 11:17:03 +0200 Subject: [PATCH] Give up waiting for the UI thread once the main loop has exited MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quitting with confirmOnQuit set hung for three seconds and printed "cannot kill child process", but only with a clean working tree. Closing the confirmation pops the context before running its handler, so the files panel is re-focused and re-renders the main view, and only then does the handler return ErrQuit. With no changed files that render is a string task, whose whole body is one hop to the UI thread — a hop that is never served, because the handler's ErrQuit has meanwhile brought the main loop down. The task can't finish, so the ViewBufferManager.Close that follows waits for it until it times out. (With changed files it's a command task instead, and every blocking point in one of those selects on the stop channel, so Close gets through.) A wait for the UI thread now ends when the loop does. That also covers the command task's own hops, which are stopped only in between them. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/gocui/gui.go | 21 +++++++++++++++++---- pkg/gocui/ui_thread_test.go | 3 --- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 43841b826..a9b68dc5c 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -38,6 +38,11 @@ var ( // ErrKeybindingNotHandled is returned when a keybinding is not handled, so that the key can be dispatched further ErrKeybindingNotHandled = standardErrors.New("keybinding not handled") + + // ErrLoopExited is returned by OnUIThreadAndWait when MainLoop has already + // returned. Nothing dequeues user events after that, so the callback it was + // asked to run on the main goroutine never will be. + ErrLoopExited = standardErrors.New("main loop exited") ) const ( @@ -897,8 +902,9 @@ func (g *Gui) EndBlockingEvents() error { // contexts) from a worker without racing the UI thread. // // The error it returns is the wait's own, never f's: it reports that f was not -// run at all. f doesn't report an error because what callers want on the UI -// thread — reading and mutating state — doesn't fail. +// run at all, which happens when the main loop has exited (ErrLoopExited). f +// doesn't report an error because what callers want on the UI thread — reading +// and mutating state — doesn't fail. // // It must be called from a worker goroutine, never from the UI thread itself: // the UI thread would block waiting for a callback only it can run, which @@ -927,8 +933,15 @@ func (g *Gui) onUIThreadAndWait(f func(), background bool) error { close(ran) return nil }) - <-ran - return nil + + select { + case <-ran: + return nil + case <-g.loopExited: + // The queue we just enqueued onto is no longer being served, so waiting + // on `ran` here would mean waiting for the rest of the process's life. + return ErrLoopExited + } } // Calls a function in a goroutine. Handles panics gracefully and tracks diff --git a/pkg/gocui/ui_thread_test.go b/pkg/gocui/ui_thread_test.go index cc5396d4a..d76bfaf9c 100644 --- a/pkg/gocui/ui_thread_test.go +++ b/pkg/gocui/ui_thread_test.go @@ -39,8 +39,5 @@ func TestOnUIThreadAndWaitGivesUpWhenTheLoopExits(t *testing.T) { }() err := resultOrTimeout(result) - /* EXPECTED: assert.ErrorIs(t, err, ErrLoopExited) - ACTUAL: */ - assert.ErrorIs(t, err, errStillWaiting) }