Does the code have any concurrent problem?

[White Pure]

Hi, 

    I wrote some code like below these days, and my colleagues and me are not sure if the code has any concurrent problem.

    Can someone help to figure out if the code has any race problem?

    Thanks very much!

    Code:

package main

import (

"sync"

"sync/atomic"

)

func main() {

var n int32

var m sync.RWMutex

go func() {

for {

atomic.LoadInt32(&n)

}

}()

go func() {

for {

m.RLock()

atomic.AddInt32(&n, 1)

m.RUnlock()

}

}()

go func() {

for {

m.Lock()

n = 0

m.Unlock()

}

}()

// do something to keep goroutines running here

......

}

    Playground link: https://play.golang.org/p/fRa09l3VQob

    

[White Pure]

Sorry for miss some code, the code should be like this:

n -= 1

m.Unlock()

}

}()

}

Playground link: https://play.golang.org/p/Mrdetw46mXR

在 2019年7月5日星期五 UTC+8下午6:40:32，White Pure写道：

[Andrew Pillar]

Have you tried building with the -race flag?

go build - race

This should help you figure out any data race conditions that may occur.

[White Pure]

Thanks for your reply. 

I've tried before and it reported race.

But I don't understand why this code has race, and what's the possible bug?

Here's the output:

$ go run -race main.go

==================

WARNING: DATA RACE

Write at 0x00c000096000 by goroutine 7:

  main.main.func3()

      /Users/purewhite/go/src/xxx/main.go:26 +0x46

Previous read at 0x00c000096000 by goroutine 5:

  sync/atomic.LoadInt32()

      /usr/local/Cellar/go/1.12.6/libexec/src/runtime/race_amd64.s:206 +0xb

  main.main.func1()

      /Users/purewhite/go/src/xxx/main.go:13 +0x38

Goroutine 7 (running) created at:

  main.main()

      /Users/purewhite/go/src/xxx/main.go:23 +0x115

Goroutine 5 (running) created at:

  main.main()

      /Users/purewhite/go/src/xxx/main.go:11 +0xbd

==================

Found 1 data race(s)

exit status 66

在 2019年7月5日星期五 UTC+8下午6:48:11，Andrew Pillar写道：

[Slawomir Pryczek]

Sure it has a race issue, because in this example you're mixing atomic operations with mutex, when you want your code to be thread safe you need to do access to the variable using mutex or using atomic in all cases. Because both ways work differently and are not "interchargable" so you can't cover access to same object using a mix of 2 ways.

You can fix your code like this:

1a. change m.RLock() to m.Lock()/m.Unlock() near atomic.AddInt32(&n, 1) because it's writing operation so you need write mutex

1b. add m.Lock()/m.Unlock near n=0 because it's writing operation as well

The, you can remove atomic ops, because now all the code is covered by mutexes

2. Change n=0 to atomic.StoreInt32(&n, 0)
Then you can remove all mutexes, because access is covered by atomic ops

[Robert Engels]

Even if you do this, the code still has a “race” as there is no way to logically reason as to the eventual outcome or purpose. You can take out all of the concurrent controls and you’ll have the same program/result (although the race detector will report an issue) - no claims can be made about any of the observed values. 

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/ed3e8643-8781-438d-8191-1c60ee7473c5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[fge...@gmail.com]

Besides what others already suggested, you'll find the reason in the
Go memory model specification:
https://golang.org/ref/mem
This part is relevant here:
"If the effects of a goroutine must be observed by another goroutine,
use a synchronization mechanism such as a lock or channel
communication to establish a relative ordering."

+1:
Instead of html in email, please use only text. Although in this
particular case the playground links would have been just enough.
(If you are curious, my email reader renders the code text with light
yellow on white background. )

> --
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-nuts...@googlegroups.com.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/golang-nuts/9c57fc61-141b-40b5-947a-5d7a3ad3547e%40googlegroups.com.

[Slawomir Pryczek]

Interesting point because output would be random but still n could be accessed by only one thread at a time, after the change ... so technically it's different kind of a race (race due to the order in which operations are scheduled because they can not be safely serialized, not due to simultaneous concurrent access due to technical error).

But sure, changing it to eg, 3 loops with are incrementing n 100k times would be much better example ;)

To unsubscribe from this group and stop receiving emails from it, send an email to golan...@googlegroups.com.