Go Wiki:代码审查:Go 并发
此页面是 Go 代码审查评论 列表的补充。此列表的目的是帮助在审查 Go 代码时发现与并发相关的错误。
您还可以只阅读一次此列表,以刷新您的记忆并确保您了解所有这些并发问题。
⚠️ 此页面由社区编写和维护。其中包含有争议的信息,可能具有误导性或不正确。
同步不足和竞争条件
- HTTP 处理程序函数是否线程安全?
- 全局函数和变量是否受互斥锁保护或以其他方式线程安全?
- 字段和变量的读取是否受保护?
- 循环变量是否作为参数传递给协程函数?
- 线程安全类型上的方法是否不返回受保护结构的指针?
Load()
或Delete()
在Load()
之后调用sync.Map
不是竞争条件?
测试
可扩展性
时间
- 使用
defer tick.Stop()
停止time.Ticker
? - 使用
Equal()
而不是==
比较time.Time
? - 在
time.Since()
的time.Time
参数中保留单调组件? - 通过
t.Before(u)
比较系统时间时,从参数中剥离单调组件?
同步不足和竞争条件
# RC.1. HTTP 处理程序函数可以从多个 goroutine 并发调用吗?很容易忽略 HTTP 处理程序应该是线程安全的,因为它们通常不会从项目代码中的任何位置显式调用,而只从 HTTP 服务器的内部调用。
# RC.2. 是否有一些字段或变量访问不受互斥锁保护,其中字段或变量是基本类型或不是明确线程安全(例如 atomic.Value
)的类型,而此字段可以从并发 goroutine 更新?即使由于非原子硬件写入和潜在的内存可见性问题,也不安全跳过对基本变量的同步读取。
另请参阅 典型的数据竞争:基本未保护变量。
# RC.3. 线程安全类型上的方法不会返回对受保护结构的指针吗?这是一个微妙的错误,会导致前面项目中描述的未受保护的访问问题。示例
type Counters struct {
mu sync.Mutex
vals map[Key]*Counter
}
func (c *Counters) Add(k Key, amount int) {
c.mu.Lock()
defer c.mu.Unlock()
count, ok := c.vals[k]
if !ok {
count = &Counter{sum: 0, num: 0}
c.vals[k] = count
}
count.sum += amount
count.n += 1
}
func (c *Counters) GetCounter(k Key) *Counter {
c.mu.Lock()
defer c.mu.Unlock()
return c.vals[k] // BUG! Returns a pointer to the structure which must be protected
}
一种可能的解决方案是在 GetCounter()
中返回一个副本,而不是结构的指针
type Counters struct {
mu sync.Mutex
vals map[Key]Counter // Note that now we are storing the Counters directly, not pointers.
}
...
func (c *Counters) GetCounter(k Key) Counter {
c.mu.Lock()
defer c.mu.Unlock()
return c.vals[k]
}
# RC.4. 如果有多个 goroutine 可以更新 sync.Map
,您不会根据先前 m.Load()
调用的成功来调用 m.Store()
或 m.Delete()
吗?换句话说,以下代码是有竞争的
var m sync.Map
// Can be called concurrently from multiple goroutines
func DoSomething(k Key, v Value) {
existing, ok := m.Load(k)
if !ok {
m.Store(k, v) // RACE CONDITION - two goroutines can execute this in parallel
... some other logic, assuming the value in `k` is now `v` in the map
}
...
}
在某些情况下,这样的竞争条件可能是良性的:例如,Load()
和 Store()
调用之间的逻辑计算要缓存在映射中的值,并且此计算总是返回相同的结果并且没有副作用。
⚠️ 可能具有误导性的信息。“竞争条件”可以指逻辑错误,例如此示例,这可能是良性的。但此短语也通常用于指代内存模型的违规,这绝不是良性的。
如果竞争条件不是良性的,请使用 sync.Map.LoadOrStore()
和 LoadAndDelete()
方法来修复它。
可扩展性
# Sc.1. 是否故意使用零容量创建通道,例如 make(chan *Foo)
?向零容量通道发送消息的 goroutine 会被阻塞,直到另一个 goroutine 接收此消息。在 make()
调用中省略容量可能只是一个错误,这会限制代码的可伸缩性,而且单元测试可能找不到此类错误。
⚠️ 具有误导性的信息。与无缓冲通道相比,缓冲通道本身不会增加“可伸缩性”。但是,缓冲通道很容易掩盖死锁和其他基本设计错误,而这些错误在无缓冲通道中会立即显现。
# Sc.2. 与普通的 sync.Mutex
相比,使用 RWMutex
进行锁定会产生额外的开销,此外,RWMutex
在 Go 中的当前实现可能存在一些 可伸缩性问题。除非情况非常明确(例如,RWMutex
用于同步每次持续数百毫秒或更长时间的许多只读操作,而需要独占锁定的写入很少发生),应该有一些基准测试证明 RWMutex
确实有助于提高性能。一个典型的示例是 RWMutex
肯定弊大于利的,它只是简单地保护结构中的变量
type Box struct {
mu sync.RWMutex // DON'T DO THIS -- use a simple Mutex instead.
x int
}
func (b *Box) Get() int {
b.mu.RLock()
defer b.mu.RUnlock()
return b.x
}
func (b *Box) Set(x int) {
b.mu.Lock()
defer b.mu.Unlock()
b.x = x
}
时间
# Tm.1. 是否使用 defer tick.Stop()
停止 time.Ticker
?当使用计时器的函数在循环中返回时,不停止计时器是一种内存泄漏。
# Tm.2. 是否使用 Equal()
方法比较 time.Time
结构,而不仅仅是 ==
?引用 time.Time
的文档
请注意,Go
==
运算符不仅比较时间戳,还比较位置和单调时钟读数。因此,Time
值不应在未首先确保为所有值设置相同位置的情况下用作映射或数据库键,这可以通过使用UTC()
或Local()
方法实现,并且通过设置t = t.Round(0)
去除单调时钟读数。通常,将t.Equal(u)
优先于t == u
,因为t.Equal()
使用最准确的可用比较,并且正确处理仅一个参数具有单调时钟读数的情况。
# Tm.3. 在调用 time.Since(t)
之前,单调组件不会从 t
中剥离?这是上一条的推论。如果在将单调组件传递给 time.Since()
函数(通过调用 UTC()
、Local()
、In()
、Round()
、Truncate()
或 AddDate()
)之前从 time.Time
结构中剥离了单调组件,则在极少数情况下,例如在最初获取开始时间和调用 time.Since()
之间通过 NTP 同步了系统时间,time.Since()
的结果可能是负数。如果没有剥离单调组件,time.Since()
将始终返回一个正持续时间。
# Tm.4. 如果您想通过 t.Before(u)
比较系统时间,是否从参数中剥离了单调组件,例如通过 u.Round(0)
?这是与 Tm.2 相关的另一点。有时,您需要仅通过其中存储的系统时间来比较两个 time.Time
结构。在将其中一个 Time
结构存储在磁盘上或通过网络发送它们之前,您可能需要这样做。例如,想象某种遥测代理,它会定期将遥测指标与时间一起推送到某个远程系统
var latestSentTime time.Time
func pushMetricPeriodically(ctx context.Context) {
t := time.NewTicker(time.Second)
defer t.Stop()
for {
select {
case <-ctx.Done: return
case <-t.C:
newTime := time.Now().Round(0) // Strip monotonic component to compare system time only
// Check that the new time is later to avoid messing up the telemetry if the system time
// is set backwards on an NTP sync.
if latestSentTime.Before(newTime) {
sendOverNetwork(NewDataPoint(newTime, metric()))
latestSentTime = newTime
}
}
}
}
如果不调用 Round(0)
,即剥离单调组件,此代码将是错误的。
阅读清单
Go 代码审查评论:一份审查 Go 代码的清单,不特定于并发。
Go 并发
- Go 内存模型
- Effective Go 中关于并发的部分
- Go 博客中的帖子
- 了解 Go 中的真实世界并发错误
并发,但不特定于 Go
此内容是 Go Wiki 的一部分。