golang中多goroutine操作map时直接赋值不用加锁？

[jinhao]

你好，
看到rand_lb.go中有这样的代码：
r.Clients在这里未加锁直接修改r.Clients = tmpClients，在其他地方会被读取，这种赋值需要加锁吧？难道是高级用法？

tmpClients := make(map[string]*WeightRpc, len(r.Clients))
        for addr, client := range r.Clients {
            if client == nil {
                continue
            }
            tmpClients[addr] = client
            if client.Addr == retryAddr {
                client.Client = rpcTmp
            }
        }
        // atomic update clients
        r.Clients = tmpClients

[Terry-Mao]

指针赋值，原子的，不需要

[Terry-Mao]

简单来说试一次COW Copy-On-Write

[jinhao]

@Terry-Mao
通过golang race detector检查，显示有data race

代码：

  1 package main
  2 import (
  3     "math/rand"
  4     "strconv"
  5     "sync"
  6 )
  7
  8 func main() {
  9     map1 := make(map[string]int)
 10     map1["100"] = 100
 11     var wg sync.WaitGroup
 12     wg.Add(1)
 13     go func() {
 14         for i := 0; i < 10000000; i++ {
 15             map2 := make(map[string]int)
 16             j := rand.Intn(30)
 17             for k := 0; k < j; k++ {
 18                 map2[strconv.Itoa(k)] = k
 19             }
 20             map1 = map2
 21         }
 22         wg.Done()
 23     }()
 24     for i := 0; i < 10; i++ {
 25         wg.Add(1)
 26         go func() {
 27             for i := 0; i < 1000000; i++ {
 28                 _, _ = map1["2"]
 29             }
 30             wg.Done()
 31         }()
 32     }
 33     wg.Wait()
 34 } 

race结果

root@ubuntu:/home# go run -race test.go
==================
WARNING: DATA RACE
Read by goroutine 6:
  main.main.func2()
      /home/test.go:28 +0x56

Previous write by goroutine 5:
  main.main.func1()
      /home/test.go:20 +0x109

Goroutine 6 (running) created at:
  main.main()
      /home/test.go:31 +0x1b7

Goroutine 5 (running) created at:
  main.main()
      /home/test.go:23 +0x167
==================
==================
WARNING: DATA RACE
Read by goroutine 6:
  runtime.mapaccess2_faststr()
      /root/go/src/runtime/hashmap_fast.go:298 +0x0
  main.main.func2()
      /home/test.go:28 +0x87

Previous write by goroutine 5:
  runtime.mapassign1()
      /root/go/src/runtime/hashmap.go:429 +0x0
  main.main.func1()
      /home/test.go:18 +0xe9

Goroutine 6 (running) created at:
  main.main()
      /home/test.go:31 +0x1b7

Goroutine 5 (running) created at:
  main.main()
      /home/test.go:23 +0x167
==================
==================
WARNING: DATA RACE
Read by goroutine 7:
  runtime.mapaccess2_faststr()
      /root/go/src/runtime/hashmap_fast.go:298 +0x0
  main.main.func2()
      /home/test.go:28 +0x87

Previous write by goroutine 5:
  runtime.mapassign1()
      /root/go/src/runtime/hashmap.go:429 +0x0
  main.main.func1()
      /home/test.go:18 +0xe9

Goroutine 7 (running) created at:
  main.main()
      /home/test.go:31 +0x1b7

Goroutine 5 (running) created at:
  main.main()
      /home/test.go:23 +0x167
==================

[yanyiwu]

谢谢作者贡献这么好的开源项目。不过个人认为是需要加锁的，之前遇到过类似的问题，贴一下相关的讨论，可以参考一下： http://yanyiwu.com/work/2015/02/07/golang-concurrency-safety.html

[Terry-Mao]

@yanyiwu data race，只是警告，不代表就真正会panic。因为编译器还是识别不出真正意图的，另外你发的链接和我这里也完全不一样，他是有一个人写，其他人在读，这块data race检查，如果不出意外，应该是根据function的栈地址不同，根据变量的读写来判断警告的，不是100%精准。

那么问题来了，我的map始终只有读。

[Terry-Mao]

我的写操作，始终只是对变量map的指针赋值，
a 如果一开始指向1，我修改成3，这个操作是原子的，即使是脏读，也不会出现影响，因为我对1，也只有读操作，那么赋值的是谁呢？是临时对象3，因为只有一个人在写他，是变量的拥有者。最终是把 a 从1 改成 3，而这部原子，这个技术大量使用在很多很多地方。

那么唯一有风险的地方是什么呢，是memory barrier。就是如果存在CPU L1/2 之类的cache，在多核下变量不更新（参考nginx的time更新），可以使用内敛汇编指示该处需要:::memory，需要屏障，另外过期内存。

然后问题又来了，golang 的memory model 不存在这个问题。

[Terry-Mao]

@yanyiwu 另外建议看看Java的ConcencyHashMap的实现，核心也是COW，就是先复制副本再替换。话说内核的fork也是COW 实现的，另外nginx的cycle 对象也是同样的远离，即变量（8字节 64bit）操作是原子的。代码中如果是直接操作 r.Clients就有问题了，我看了weedfs的代码，他就犯错了，再另外，Java的Hashmap为什么他们都犯错了，都是用的同一个对象（那怕只有一个人写），也是不行的，

[Terry-Mao]

你会发现ConcencyHashMap 为什么需要volatile来做这个事情，这就是memory barrier的故事，C代码需要依赖内联汇编，让其他的pthread可见更新asm volatile ("" ::: "memory")，我之前担心golang会有类似问题，后来看了内存模型，确认不会出现。

[Terry-Mao]

再另外：
https://github.com/aszxqw/practice/blob/master/go/rwmutex/notlock.go
https://github.com/aszxqw/practice/blob/master/go/rwmutex/lock.go

你的两个例子，知道为啥有问题吗？

        g = Tmp{rand.Intn(100), strconv.Itoa(i), rand.Float64()}

因为这一步不是原子的。

代码修改：var g = new(Tmp)
g = &Tmp{rand.Intn(100), strconv.Itoa(i), rand.Float64()}

就会是预期的，Have Fun！

[yanyiwu]

感谢这么详细的回复。

之前确实没仔细看原来你是COW实现，你说的COW的实现我明白，也认为是ok的，这个无需多言。
但是主要的分歧在于，我的观点在于那个指针赋值并不能确保是原子的。

按我看你的说法来看，我理解你要表达的意思是『对于map指针的赋值，是原子的，是不会出问题的，你通过看golang的内存模型确认了这个是不会出问题的』我这么理解对吗？

假设我对你的评论理解是对的，那么有个地方关于指针赋值是否是原子的有对应的讨论：

http://stackoverflow.com/questions/21447463/is-assigning-a-pointer-atomic-in-golang

里面回答是指针赋值不能确保是原子行为，
即使目前的实现看来是原子的，但是无法保证后来的golang源码改动后仍然是原子的。

Since the spec doesn't specify you should assume it is not. Even if it is currently atomic it's possible that it could change without ever violating the spec.

[Terry-Mao]

Okay，如果以后会改动，我会注意下的，之前看过6g的汇编，确认过应该是只有一次指令，具体可以go tool asm 看下。多谢提醒，如果以后会改动会坑了不少人。。。

[yanyiwu]

@Terry-Mao 点个赞。再次感谢开发出这么好的项目，辛苦了。

[Terry-Mao]

另外建议楼上两位可以看下goim代码，gopush好久不维护了，goim应该是在GC和CPU做了大量变态优化，另外我观察到你们有关注weedfs，可以直接看看我的bfs。bfs在内核的IO优化理解，超过weedfs：）

[Terry-Mao]

还有，可以加一下我的联系方式，经常讨论：83873308

[loxp]

我的写操作，始终只是对变量map的指针赋值，
a 如果一开始指向1，我修改成3，这个操作是原子的，即使是脏读，也不会出现影响，因为我对1，也只有读操作，那么赋值的是谁呢？是临时对象3，因为只有一个人在写他，是变量的拥有者。最终是把 a 从1 改成 3，而这部原子，这个技术大量使用在很多很多地方。

那么唯一有风险的地方是什么呢，是memory barrier。就是如果存在CPU L1/2 之类的cache，在多核下变量不更新（参考nginx的time更新），可以使用内敛汇编指示该处需要:::memory，需要屏障，另外过期内存。

然后问题又来了，golang 的memory model 不存在这个问题。

https://golang.org/ref/mem#tmp_10

type T struct {
	msg string
}

var g *T

func setup() {
	t := new(T)
	t.msg = "hello, world"
	g = t
}

func main() {
	go setup()
	for g == nil {
	}
	print(g.msg)
}

Even if main observes g != nil and exits its loop, there is no guarantee that it will observe the initialized value for g.msg.
In all these examples, the solution is the same: use explicit synchronization.

类似的操作, 是否会有可见性问题?

[yindaren]

你会发现ConcencyHashMap 为什么需要volatile来做这个事情，这就是memory barrier的故事，C代码需要依赖内联汇编，让其他的pthread可见更新asm volatile ("" ::: "memory")，我之前担心golang会有类似问题，后来看了内存模型，确认不会出现。

请问下为啥go不需要memory barrier？是怎么保证可见性的？

[JerxLuo]

你会发现ConcencyHashMap 为什么需要volatile来做这个事情，这就是memory barrier的故事，C代码需要依赖内联汇编，让其他的pthread可见更新asm volatile ("" ::: "memory")，我之前担心golang会有类似问题，后来看了内存模型，确认不会出现。

我也担心golang有内存屏障的问题，你说不会出现是为什么呢？

[keyganker]

Okay，如果以后会改动，我会注意下的，之前看过6g的汇编，确认过应该是只有一次指令，具体可以go tool asm 看下。多谢提醒，如果以后会改动会坑了不少人。。。

我也觉得不会改的，否则坑死一堆项目，或者出现一批打死也不升级的项目。。

[dukeduffff]

指针赋值没见到汇编之前一定要认为是非原子的，也就是说指针赋值这里并没有内存屏障，没有内存屏障就无法保证赋值之前的所有操作全部完成

[Amoteamame]

再另外：
https://github.com/aszxqw/practice/blob/master/go/rwmutex/notlock.go
https://github.com/aszxqw/practice/blob/master/go/rwmutex/lock.go

你的两个例子，知道为啥有问题吗？

        g = Tmp{rand.Intn(100), strconv.Itoa(i), rand.Float64()}

因为这一步不是原子的。

代码修改：var g = new(Tmp)
g = &Tmp{rand.Intn(100), strconv.Itoa(i), rand.Float64()}

就会是预期的，Have Fun！

今天看到这个issue，试了下，不是原子的，必须加锁。

[singchia]

建议 atomic.SwapPointer + unsafe.Pointer

package main

import (
    "math/rand"
    "strconv"
    "sync"
    "sync/atomic"
    "unsafe"
)

func main() {
    m1 := map[string]int{"100": 100}
    p1 := unsafe.Pointer(&m1)
    map1 := &p1
    var wg sync.WaitGroup
    wg.Add(1)
    go func() {
        for i := 0; i < 10000000; i++ {
            m2 := make(map[string]int)
            j := rand.Intn(30)
            for k := 0; k < j; k++ {
                m2[strconv.Itoa(k)] = k
            }
            atomic.SwapPointer(map1, unsafe.Pointer(&m2))
        }
        wg.Done()
    }()
    for i := 0; i < 10; i++ {
        wg.Add(1)
        go func() {
            p1 := atomic.LoadPointer(map1)
            for i := 0; i < 10000000; i++ {
                _, _ = (*(*map[string]int)(p1))["2"]
            }
            wg.Done()
        }()
    }
    wg.Wait()
}