diff --git a/pkg/statushooks/spinner.go b/pkg/statushooks/spinner.go index 91475c353..b3d0ce75f 100644 --- a/pkg/statushooks/spinner.go +++ b/pkg/statushooks/spinner.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "strings" + "sync" "time" "github.com/briandowns/spinner" @@ -28,6 +29,7 @@ type StatusSpinner struct { cancel chan struct{} delay time.Duration visible bool + mu sync.RWMutex // protects spinner.Suffix and visible fields } type StatusSpinnerOpt func(*StatusSpinner) @@ -92,7 +94,9 @@ func (s *StatusSpinner) Warn(msg string) { // Hide implements StatusHooks func (s *StatusSpinner) Hide() { + s.mu.Lock() s.visible = false + s.mu.Unlock() if s.cancel != nil { close(s.cancel) } @@ -100,6 +104,8 @@ func (s *StatusSpinner) Hide() { } func (s *StatusSpinner) Show() { + s.mu.Lock() + defer s.mu.Unlock() s.visible = true if len(strings.TrimSpace(s.spinner.Suffix)) > 0 { // 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 func (s *StatusSpinner) UpdateSpinnerMessage(newMessage string) { newMessage = s.truncateSpinnerMessageToScreen(newMessage) + s.mu.Lock() + defer s.mu.Unlock() s.spinner.Suffix = fmt.Sprintf(" %s", newMessage) // if the spinner is not active, start it if s.visible && !s.spinner.Active() { diff --git a/pkg/statushooks/statushooks_test.go b/pkg/statushooks/statushooks_test.go index 220399c48..37f14fc4c 100644 --- a/pkg/statushooks/statushooks_test.go +++ b/pkg/statushooks/statushooks_test.go @@ -55,7 +55,7 @@ func TestSpinnerConcurrentShowHide(t *testing.T) { // TestSpinnerConcurrentUpdate tests concurrent message updates for race conditions // BUG: This exposes a race condition on spinner.Suffix field 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.Show() defer spinner.Hide() @@ -242,7 +242,7 @@ func TestSpinnerUpdateAfterHide(t *testing.T) { // TestSpinnerSetStatusRace tests concurrent SetStatus calls 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.Show() @@ -327,7 +327,7 @@ func TestSpinnerMultipleStartStopCycles(t *testing.T) { // TestSpinnerConcurrentSetStatusAndHide tests race between SetStatus and Hide 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.Show()