From 86b5174b62b1c2c4935247c5f0807887d41c1b1e Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Tue, 1 Oct 2024 14:20:09 +1000 Subject: [PATCH] fix: panic on exit (#2908) fixes: #2902 --------- Co-authored-by: github-actions[bot] --- internal/terminal/interactive.go | 24 ++++++++++++++---------- internal/terminal/status.go | 17 +++++++---------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/internal/terminal/interactive.go b/internal/terminal/interactive.go index ef174709e3..a7c59fd4b9 100644 --- a/internal/terminal/interactive.go +++ b/internal/terminal/interactive.go @@ -20,7 +20,6 @@ import ( const interactivePrompt = "\033[32m>\033[0m " var _ readline.AutoCompleter = &FTLCompletion{} -var errExitTrap = errors.New("exit trap") type KongContextBinder func(ctx context.Context, kctx *kong.Context) context.Context @@ -53,7 +52,19 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext }) l.CaptureExitSignal() // Overload the exit function to avoid exiting the process - k.Exit = func(i int) { panic(errExitTrap) } + existing := k.Exit + k.Exit = func(i int) { + if i != 0 { + _ = l.Close() + if existing == nil { + // Should not happen, but no harm being cautious + os.Exit(i) + } + existing(i) + } + // For a normal exit from an interactive command we just ignore it + // This usually comes from --help or --version + } for { line, err := l.Readline() if errors.Is(err, readline.ErrInterrupt) { @@ -77,14 +88,6 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext continue } func() { - defer func() { - // Catch Exit() and continue the loop - if r := recover(); r != nil { - if r == errExitTrap { //nolint:errorlint - return - } - } - }() kctx, err := k.Parse(args) if err != nil { errorf("%s", err) @@ -99,6 +102,7 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext } }() } + _ = l.Close() return nil } diff --git a/internal/terminal/status.go b/internal/terminal/status.go index 1afeda034e..a1a9c9507d 100644 --- a/internal/terminal/status.go +++ b/internal/terminal/status.go @@ -94,7 +94,7 @@ type terminalStatusManager struct { write *os.File closed atomic.Value[bool] totalStatusLines int - statusLock sync.RWMutex + statusLock sync.Mutex lines []*terminalStatusLine moduleLine *terminalStatusLine moduleStates map[string]BuildState @@ -118,7 +118,7 @@ func NewStatusManager(ctx context.Context) StatusManager { if err != nil { return &noopStatusManager{} } - sm := &terminalStatusManager{statusLock: sync.RWMutex{}, moduleStates: map[string]BuildState{}, height: height, width: width, exitWait: sync.WaitGroup{}} + sm := &terminalStatusManager{statusLock: sync.Mutex{}, moduleStates: map[string]BuildState{}, height: height, width: width, exitWait: sync.WaitGroup{}} sm.exitWait.Add(1) sm.old = os.Stdout sm.oldErr = os.Stderr @@ -257,10 +257,6 @@ func IsANSITerminal(ctx context.Context) bool { return ok } -func (r *terminalStatusManager) gotoCoords(line int, col int) { - r.underlyingWrite(fmt.Sprintf("\033[%d;%dH", line, col)) -} - func (r *terminalStatusManager) clearStatusMessages() { if r.totalStatusLines == 0 { return @@ -359,8 +355,8 @@ func (r *terminalStatusManager) Close() { } func (r *terminalStatusManager) writeLine(s string, last bool) { - r.statusLock.RLock() - defer r.statusLock.RUnlock() + r.statusLock.Lock() + defer r.statusLock.Unlock() if !last { s += "\n" } @@ -371,7 +367,9 @@ func (r *terminalStatusManager) writeLine(s string, last bool) { } r.clearStatusMessages() r.underlyingWrite("\r" + s) - r.redrawStatus() + if !last { + r.redrawStatus() + } } func (r *terminalStatusManager) redrawStatus() { @@ -476,7 +474,6 @@ type terminalStatusLine struct { func (r *terminalStatusLine) Close() { r.manager.statusLock.Lock() defer r.manager.statusLock.Unlock() - for i := range r.manager.lines { if r.manager.lines[i] == r { r.manager.lines = append(r.manager.lines[:i], r.manager.lines[i+1:]...)