Fix #4744: Race condition on spinner.Suffix field (#4772)

This commit is contained in:
Nathan Wallace
2025-11-11 18:01:56 +08:00
committed by GitHub
parent da0c9ddc0e
commit 6ebc0e5040
2 changed files with 11 additions and 3 deletions

View File

@@ -4,6 +4,7 @@ import (
"fmt" "fmt"
"os" "os"
"strings" "strings"
"sync"
"time" "time"
"github.com/briandowns/spinner" "github.com/briandowns/spinner"
@@ -28,6 +29,7 @@ type StatusSpinner struct {
cancel chan struct{} cancel chan struct{}
delay time.Duration delay time.Duration
visible bool visible bool
mu sync.RWMutex // protects spinner.Suffix and visible fields
} }
type StatusSpinnerOpt func(*StatusSpinner) type StatusSpinnerOpt func(*StatusSpinner)
@@ -92,7 +94,9 @@ func (s *StatusSpinner) Warn(msg string) {
// Hide implements StatusHooks // Hide implements StatusHooks
func (s *StatusSpinner) Hide() { func (s *StatusSpinner) Hide() {
s.mu.Lock()
s.visible = false s.visible = false
s.mu.Unlock()
if s.cancel != nil { if s.cancel != nil {
close(s.cancel) close(s.cancel)
} }
@@ -100,6 +104,8 @@ func (s *StatusSpinner) Hide() {
} }
func (s *StatusSpinner) Show() { func (s *StatusSpinner) Show() {
s.mu.Lock()
defer s.mu.Unlock()
s.visible = true s.visible = true
if len(strings.TrimSpace(s.spinner.Suffix)) > 0 { if len(strings.TrimSpace(s.spinner.Suffix)) > 0 {
// only show the spinner if there's an actual message to show // only show the spinner if there's an actual message to show
@@ -110,6 +116,8 @@ func (s *StatusSpinner) Show() {
// UpdateSpinnerMessage updates the message of the given spinner // UpdateSpinnerMessage updates the message of the given spinner
func (s *StatusSpinner) UpdateSpinnerMessage(newMessage string) { func (s *StatusSpinner) UpdateSpinnerMessage(newMessage string) {
newMessage = s.truncateSpinnerMessageToScreen(newMessage) newMessage = s.truncateSpinnerMessageToScreen(newMessage)
s.mu.Lock()
defer s.mu.Unlock()
s.spinner.Suffix = fmt.Sprintf(" %s", newMessage) s.spinner.Suffix = fmt.Sprintf(" %s", newMessage)
// if the spinner is not active, start it // if the spinner is not active, start it
if s.visible && !s.spinner.Active() { if s.visible && !s.spinner.Active() {

View File

@@ -55,7 +55,7 @@ func TestSpinnerConcurrentShowHide(t *testing.T) {
// TestSpinnerConcurrentUpdate tests concurrent message updates for race conditions // TestSpinnerConcurrentUpdate tests concurrent message updates for race conditions
// BUG: This exposes a race condition on spinner.Suffix field // BUG: This exposes a race condition on spinner.Suffix field
func TestSpinnerConcurrentUpdate(t *testing.T) { func TestSpinnerConcurrentUpdate(t *testing.T) {
t.Skip("Demonstrates bugs #4743, #4744 - Race condition in concurrent Update. Remove this skip in bug fix PR commit 1, then fix in commit 2.") // t.Skip("Demonstrates bugs #4743, #4744 - Race condition in concurrent Update. Remove this skip in bug fix PR commit 1, then fix in commit 2.")
spinner := NewStatusSpinnerHook() spinner := NewStatusSpinnerHook()
spinner.Show() spinner.Show()
defer spinner.Hide() defer spinner.Hide()
@@ -242,7 +242,7 @@ func TestSpinnerUpdateAfterHide(t *testing.T) {
// TestSpinnerSetStatusRace tests concurrent SetStatus calls // TestSpinnerSetStatusRace tests concurrent SetStatus calls
func TestSpinnerSetStatusRace(t *testing.T) { func TestSpinnerSetStatusRace(t *testing.T) {
t.Skip("Demonstrates bugs #4743, #4744 - Race condition in SetStatus. Remove this skip in bug fix PR commit 1, then fix in commit 2.") // t.Skip("Demonstrates bugs #4743, #4744 - Race condition in SetStatus. Remove this skip in bug fix PR commit 1, then fix in commit 2.")
spinner := NewStatusSpinnerHook() spinner := NewStatusSpinnerHook()
spinner.Show() spinner.Show()
@@ -327,7 +327,7 @@ func TestSpinnerMultipleStartStopCycles(t *testing.T) {
// TestSpinnerConcurrentSetStatusAndHide tests race between SetStatus and Hide // TestSpinnerConcurrentSetStatusAndHide tests race between SetStatus and Hide
func TestSpinnerConcurrentSetStatusAndHide(t *testing.T) { func TestSpinnerConcurrentSetStatusAndHide(t *testing.T) {
t.Skip("Demonstrates bugs #4743, #4744 - Race condition in concurrent SetStatus and Hide. Remove this skip in bug fix PR commit 1, then fix in commit 2.") // t.Skip("Demonstrates bugs #4743, #4744 - Race condition in concurrent SetStatus and Hide. Remove this skip in bug fix PR commit 1, then fix in commit 2.")
spinner := NewStatusSpinnerHook() spinner := NewStatusSpinnerHook()
spinner.Show() spinner.Show()