Golang Data Race, with 66 exit status

Question

I have the following code, and I am having data race. The Round function periodically check runs the function to delete the contents of the map As I read here: Is it safe to remove selected keys from Golang map within a range loop?

Removing the data from map is safe, but I am having data race

package main

import (
    "fmt"
    "sync"
    "time"
)

type City struct {
    ID string
}

type Map struct {
    sync.RWMutex
    Data map[string]City
}

var done = make(chan struct{})

func (m *Map) Round() {
    for {
        select {
        case <-time.After(2 * time.Second):
            for i, v := range m.Data {
                fmt.Println("-----", v)
                delete(m.Data, i)
            }
        case <-done:
            println("bye")
            break
        }
    }
}

func (m *Map) Add(id string, h City) {
    m.Lock()
    m.Data[id] = h
    m.Unlock()
}

func main() {
    m := Map{}
    m.Data = make(map[string]City)

    m.Data["Ottowa"] = City{"Canada"}
    m.Data["London"] = City{"GB"}
    m.Data["malafya"] = City{"malafya"}

    go m.Round()

    for i := 0; i < 4; i++ {
        go func() {
            time.Sleep(2 * time.Second)
            go m.Add("uz", City{"CityMakon"})
            go m.Add("uzb", City{"CityMakon"})
        }()
    }
    time.Sleep(5 * time.Second)
    done <- struct{}{}
}

the output:

----- {Canada}
----- {GB}
----- {malafya}
==================
WARNING: DATA RACE
Write by goroutine 12:
  runtime.mapassign1()
      /usr/lib/golang/src/runtime/hashmap.go:411 +0x0
  main.(*Map).Add()
      /home/narkoz/elixir/round.go:37 +0xaa

Previous write by goroutine 6:
  runtime.mapdelete()
      /usr/lib/golang/src/runtime/hashmap.go:511 +0x0
  main.(*Map).Round()
      /home/narkoz/elixir/round.go:26 +0x3a9

Goroutine 12 (running) created at:
  main.main.func1()
      /home/narkoz/elixir/round.go:54 +0x8c

Goroutine 6 (running) created at:
  main.main()
      /home/narkoz/elixir/round.go:49 +0x2af
==================
----- {CityMakon}
----- {CityMakon}
Found 1 data race(s)
exit status 66

But when I change the value type of the map to int or string, there is not data race.

What solution do you recommend ?

I think that's coincidence about the types affecting results (but maybe someone who knows better will reply). You need locking around the map iteration and delete in `Round()` too. — twotwotwo, Jun 10 '16 at 18:50
Thanks, so I have wrapped the range loop with RLock and RUnlock, this removed the second data race — maksadbek, Jun 10 '16 at 19:05
The race detector tells you _exactly_ where the race occurred. You're calling `delete` without taking the lock. — JimB, Jun 10 '16 at 19:10
@maksadbek Since there's a `delete` inside that loop, you really need a `Lock`, not just an `RLock`. — twotwotwo, Jun 10 '16 at 19:13

ivan.sim · Accepted Answer · 2016-06-15T19:55:13.900

UPDATES:

But when I change the value type of the map to int or string, there is not data race.

I tested your code further. Changing the map's value type to int or string continues to yield the race condition. Try run it in a while loop on your shell, you will see what I mean:

$ while true; do go run -race main.go; done

There shouldn't be any differences between the value types.

As reported by the race detectors, there are two different race conditions. The first race (which you've fixed) happens between the read (of i) at line 54 and the write (to i) at line 51. This happens because your goroutine closure holds a reference to i, which is changed by the for loop in your main goroutine. You can resolve it by either getting rid of the println(">>", i) or passing i into your closure like this:

for i := 0; i < 4; i++ {
  go func(index int) {
    time.Sleep(2 * time.Second)
    println(">>", index)
    go m.Add("uz", City{"CityMakon"})
    go m.Add("uzb", City{"CityMakon"})
  }(i)
}

The second race condition happens between the assignment on line 37 (m.Data[id] = h) and the delete in line 25 (delete(m.Data, i)). The race detector marks this as a race condition because it can't guarantee the Happen Before constraint on your code. You can resolve this by either:

Locking the delete statement:

m.Lock()
delete(m.Data, i)
m.Unlock()

Or alternately, extract the two cases in your Round() method into two methods, ranging over the channel:

func (m *Map) Round() {
  for i, v := range m.Data {
    fmt.Println("-----", v)
    delete(m.Data, i)
  }
}

func (m *Map) Done() {
  for range done {
    println("bye")
    break
  }
}

func main() {
  // ...
  go Round()
  go Done()
}

Thanks, this solved one data race of two. I updated it. But the second data race one still occurs — maksadbek, Jun 10 '16 at 19:00
(Noting the second race seems to be resolved as well, per your reply to my comment on the question.) — twotwotwo, Jun 10 '16 at 19:09

Golang Data Race, with 66 exit status

1 Answers1