spinner icon indicating copy to clipboard operation
spinner copied to clipboard

There is a race issue

Open metrue opened this issue 4 years ago • 6 comments

How to produces?

spinner.go

package spinner

import (
	"time"

	"github.com/briandowns/spinner"
)

var s *spinner.Spinner

func init() {
	style := spinner.CharSets[36]
	interval := 100 * time.Millisecond
	s = spinner.New(style, interval)
}

// Start spinner
func Start(task string) {
	s.Start()
}

// Stop spinner
func Stop(task string, err error) {
	s.Stop()
}

spinner_test.go

package spinner

import (
	"fmt"
	"testing"
	"time"
)

func TestSpinner(t *testing.T) {
	t.Run("failure", func(t *testing.T) {
		Start("task 2")
		time.Sleep(1 * time.Second)
		Stop("task 2", fmt.Errorf("error happened"))
	})
	t.Run("success", func(t *testing.T) {
		Start("task 1")
		time.Sleep(1 * time.Second)
		Stop("task 1", nil)
	})
}

Then run test command,

go test -v ./... --race

Will get failure message.

=== RUN   TestSpinner
=== RUN   TestSpinner/failure
=== RUN   TestSpinner/success
==================
WARNING: DATA RACE
Read at 0x00c000140340 by goroutine 9:
  github.com/briandowns/spinner.(*Spinner).Start.func1()
      /Users/minhuang/.go/pkg/mod/github.com/briandowns/[email protected]/spinner.go:288 +0x109

Previous write at 0x00c000140340 by goroutine 11:
  github.com/briandowns/spinner.(*Spinner).Start()
      /Users/minhuang/.go/pkg/mod/github.com/briandowns/[email protected]/spinner.go:278 +0xa7
  github.com/metrue/fx/pkg/spinner.Start()
      /Users/minhuang/Codes/fx/pkg/spinner/spiner.go:19 +0x4b
  github.com/metrue/fx/pkg/spinner.TestSpinner.func2()
      /Users/minhuang/Codes/fx/pkg/spinner/spiner_test.go:16 +0x2b
  testing.tRunner()
      /usr/local/opt/go/libexec/src/testing/testing.go:992 +0x1eb

Goroutine 9 (running) created at:
  github.com/briandowns/spinner.(*Spinner).Start()
      /Users/minhuang/.go/pkg/mod/github.com/briandowns/[email protected]/spinner.go:281 +0xef
  github.com/metrue/fx/pkg/spinner.Start()
      /Users/minhuang/Codes/fx/pkg/spinner/spiner.go:19 +0x4f
  github.com/metrue/fx/pkg/spinner.TestSpinner.func1()
      /Users/minhuang/Codes/fx/pkg/spinner/spiner_test.go:11 +0x2f
  testing.tRunner()
      /usr/local/opt/go/libexec/src/testing/testing.go:992 +0x1eb

Goroutine 11 (running) created at:
  testing.(*T).Run()
      /usr/local/opt/go/libexec/src/testing/testing.go:1043 +0x660
  github.com/metrue/fx/pkg/spinner.TestSpinner()
      /Users/minhuang/Codes/fx/pkg/spinner/spiner_test.go:15 +0x8c
  testing.tRunner()
      /usr/local/opt/go/libexec/src/testing/testing.go:992 +0x1eb
==================
    TestSpinner/success: testing.go:906: race detected during execution of test
    TestSpinner: testing.go:906: race detected during execution of test
--- FAIL: TestSpinner (2.01s)
    --- PASS: TestSpinner/failure (1.00s)
    --- FAIL: TestSpinner/success (1.00s)
    : testing.go:906: race detected during execution of test
FAIL
FAIL	github.com/metrue/fx/pkg/spinner	2.039s
FAIL

metrue avatar May 07 '20 11:05 metrue

Thanks for the report. I'll take a look over the next day or two.

briandowns avatar May 07 '20 16:05 briandowns

I also encountered this issue in v1.11.1.

But after investigating, I see that the issue was fixed here in July.

But unfortunately this fix did not make it into v1.11.1. Is there a timeline for the next release which would contain the fix from July?

clebs avatar Nov 20 '20 08:11 clebs

I'm curious about the next release for this reason as well, thanks.

djscholl avatar Nov 30 '20 00:11 djscholl

I just cut v1.12.0 this evening.

briandowns avatar Nov 30 '20 02:11 briandowns

too many data race issues. Try it with the -race flag on:

$ go run -race main.go
package main

import (
	"time"

	"github.com/briandowns/spinner"
)

func dataRace1() {
	sp := spinner.New(spinner.CharSets[4], time.Millisecond*200)
	sp.Color("blue")
	sp.Prefix = "test"
}

func dataRace2() {
	sp := spinner.New(spinner.CharSets[4], time.Millisecond*200)
	sp.Color("blue")
	sp.Prefix = "test2"
}

func dataRace3() {
	sp := spinner.New(spinner.CharSets[4], time.Millisecond*200)
	sp.Start()
	sp.Prefix = "test2"
}

func dataRace4() {
	sp := spinner.New(spinner.CharSets[4], time.Millisecond*200)
	sp.Color("blue")
	sp.Color("red")
}

func main() {
	dataRace1()
	dataRace2()
	dataRace3()
	dataRace4()
}

Pantani avatar Jun 21 '21 21:06 Pantani

@metrue @Pantani I think this is ultimately going to remain a problem, until the API is changed so that methods are used to update settings and not struct fields directly. The data race issues were a big reason I created this alternative spinner, after realizing fixing these issues would be a pretty massive change for this package.

I would love it if you could give it a go, and let me know if you experience any issues: https://github.com/theckman/yacspin

theckman avatar Dec 11 '21 07:12 theckman