0

I have a hard time understanding concurrency/paralel. in my code I made a loop of 5 cycle. Inside of the loop I added the wg.Add(1), in total I have 5 Adds. Here's the code:

package main

import (
    "fmt"
    "sync"
)

func main() {
    var list []int
    wg := sync.WaitGroup{}
    for i := 0; i < 5; i++ {
        wg.Add(1)
        go func(c *[]int, i int) {
            *c = append(*c, i)
            wg.Done()
        }(&list, i)
    }
    wg.Wait()
    fmt.Println(len(list))
}

The main func waits until all the goroutines finish but when I tried to print the length of slice I get random results. ex (1,3,etc) is there something that is missing for it to get the expected result ie 5 ?

icza
  • 389,944
  • 63
  • 907
  • 827
  • 7
    You have a data race, you can't write to a value concurrently in multiple goroutines. There is no expected output. – JimB Aug 03 '18 at 15:04
  • write a test, or compile with the race detector enabled. you're using a pointer to the same slice in all routines. That's actually quite unsafe. The go runtime might reallocate and move the slice when you call `append`, that's the whole reason why you have to reassing the slice every time you call append – Elias Van Ootegem Aug 03 '18 at 19:01
  • @HelloMyNameIs Your array example is explained here: [Can I concurrently write different slice elements](https://stackoverflow.com/questions/49879322/can-i-concurrently-write-different-slice-elements/49879469#49879469) – icza Aug 03 '18 at 19:36
  • @icza yes that clears it, thx – HelloMyNameIs Aug 03 '18 at 21:18

1 Answers1

1

is there something that is missing for it to get the expected result ie 5 ?

Yes, proper synchronization. If multiple goroutines access the same variable where at least one of them is a write, you need explicit synchronization.

Your example can be "secured" with a single mutex:

var list []int
wg := sync.WaitGroup{}

mu := &sync.Mutex{} // A mutex

for i := 0; i < 5; i++ {
    wg.Add(1)
    go func(c *[]int, i int) {
        mu.Lock() // Must lock before accessing shared resource
        *c = append(*c, i)
        mu.Unlock() // Unlock when we're done with it
        wg.Done()
    }(&list, i)
}
wg.Wait()

fmt.Println(len(list))

This will always print 5.

Note: the same slice is read at the end to prints its length, yet we are not using the mutex there. This is because the use of waitgroup ensures that we can only get to that point after all goroutines that modify it have completed their job, so data race cannot occur there. But in general both reads and writes have to be synchronized.

See possible duplicates:

go routine not collecting all objects from channel

Server instances with multiple users

Why does this code cause data race?

How safe are Golang maps for concurrent Read/Write operations?

golang struct concurrent read and write without Lock is also running ok?

See related questions:

Can I concurrently write different slice elements

If I am using channels properly should I need to use mutexes?

Is it safe to read a function pointer concurrently without a lock?

Concurrent access to maps with 'range' in Go

icza
  • 389,944
  • 63
  • 907
  • 827
  • Shouldn't data race cause a panic? Can it produce incorrect result without panicing? – Uvelichitel Aug 03 '18 at 15:24
  • @Uvelichitel The runtime does not check for data race, so it doesn't know when it happens, unless you pass the `-race` param to the go tool. On the other hand the runtime does detect some misuse of maps, which crashes the app. But generally data races "don't panic". – icza Aug 03 '18 at 15:36
  • Shouldn't a system detect and prevent incorrect memory access on OS level? – Uvelichitel Aug 03 '18 at 15:40
  • @Uvelichitel This isn't an OS-level incorrect memory access. The app only accesses its own memory, but its "internal" goroutines are not synchronized, and the runtime does not guarantee valid results. And the runtime does not detect it because it significantly increases the memory and computation requirements. It only does this if you explicitly ask it to (using the `-race` flag), which is usually only done during tests, but not during "normal" operation. – icza Aug 03 '18 at 15:46
  • @icza i see, does this rule also apply when i want to process(read/write) singular key from a map ? ex. goroutine 1 process map["first"], and goroutine 2 process map["second"] etc. Do i need to wait for the goroutine 1 to finish to start the second one because they read/write different key but still in the same variable ? – HelloMyNameIs Aug 03 '18 at 16:08
  • @HelloMyNameIs Yes, if the same map is read / written from multiple goroutines, access to the map itself must be synchronized. See related question: [Concurrent access to maps with 'range' in Go](https://stackoverflow.com/questions/40442846/concurrent-access-to-maps-with-range-in-go/40456170#40456170) – icza Aug 03 '18 at 16:21