Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

There is a race issue #99

Open
metrue opened this issue May 7, 2020 · 6 comments
Open

There is a race issue #99

metrue opened this issue May 7, 2020 · 6 comments

Comments

@metrue
Copy link

metrue commented May 7, 2020

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/spinner@v1.11.1/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/spinner@v1.11.1/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/spinner@v1.11.1/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
@briandowns
Copy link
Owner

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

@clebs
Copy link

clebs commented Nov 20, 2020

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?

@djscholl
Copy link

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

@briandowns
Copy link
Owner

I just cut v1.12.0 this evening.

@Pantani
Copy link

Pantani commented Jun 21, 2021

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()
}

@theckman
Copy link

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants