search

franela / goblin

Minimal and Beautiful Go testing framework
MIT License
884 stars 79 forks source link

Concurrency race #98

Closed mikebellcoder closed 3 years ago

mikebellcoder commented 3 years ago

Bug

When using Goblin in Go table tests run in parallel occasionally will get the following error, a large dump of data/stack and failed test:

fatal error: concurrent map writes
...

Why

When using Goblin in parallel it makes multiple calls to flag.Parse() which somewhere does a map assignment without a mutex. flag.Parse() is not meant to be called multiple times like this.

Solution

Wrapping the call flag.Parse() in sync.Once.Do() so it only gets called once per instance of Goblin and resolves the issue.

marcosnils commented 3 years ago

hey @beenbell88! Thx for the contribution. Seems like tests are failing in the parseFlags function we're trying to fix here?

mikebellcoder commented 3 years ago

@marcosnils Yes. I we are using Goblin were I work and every now and then it fails due to race condition. I have a test in race_test.go TestG_Concurrency_Race which shows the error we were getting. I added some mutex lock/unlocks in places where the internal one could not be used and this allowed my test to pass but now a single test suite in goblin_test.go TestAsync is failing and I'm trying to solve that, but it seems weird that as I execute it again and again I start getting different results, i.e. different combinations of assertions in that test are failing.