Go Wiki: 代码审查:Go 并发

本页面是对 Go 代码审查注释 列表的补充。此列表的目的是帮助在审查 Go 代码时发现并发相关的错误。

您也可以只通读一遍此列表,以刷新您的记忆,并确保您了解所有这些并发陷阱。

⚠️ 本页面由社区编写和维护。其中包含有争议的信息,可能具有误导性或不正确。


同步不足和竞态条件

测试

可伸缩性

时间


同步不足和竞态条件

# 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.Store()m.Delete() 来依赖于之前 m.Load() 调用的成功?** 换句话说,以下代码是有竞态的

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() 调用之间的逻辑计算要缓存到 map 中的值,而该计算始终返回相同的结果且没有副作用。

⚠️ **潜在的误导性信息**。“竞态条件”可以指逻辑错误,例如这个例子,它可以是良性的。但这个短语也常用于指违反内存模型,这永远不是良性的。

如果竞态条件不是良性的,请使用 sync.Map.LoadOrStore()LoadAndDelete() 方法来修复它。

可伸缩性

# Sc.1. **故意创建一个零容量的通道(channel)是否是故意的**,例如 make(chan *Foo)?向零容量通道发送消息的 goroutine 会一直阻塞,直到另一个 goroutine 接收到该消息。在 make() 调用中省略容量可能只是一个错误,它会限制代码的可伸缩性,并且单元测试很可能找不到这种错误。

⚠️ **误导性信息**。缓冲通道本身并不比无缓冲通道更能提高“可伸缩性”。但是,缓冲通道很容易掩盖死锁和其他基本设计错误,而这些错误在使用无缓冲通道时会立即显现。

# Sc.2. 与纯粹的 sync.Mutex 相比,使用 RWMutex 进行锁定会产生额外的开销,此外,Go 中 RWMutex 的当前实现可能存在一些 可伸缩性问题。除非情况非常明确(例如,使用 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?** 当使用该 tick 的函数在循环中返回时,不停止 tick 会导致内存泄漏。

# Tm.2. **是否使用 Equal() 方法而不是简单的 == 来比较 time.Time 结构体?** 引用 time.Time 的文档

请注意,Go 的 == 运算符不仅比较时间点,还比较 Location 和单调时钟读数。因此,在确保所有值都设置了相同的 Location(可以通过使用 UTC()Local() 方法实现)并且已剥离单调时钟读数(通过设置 t = t.Round(0))之前,不应将 Time 值用作 map 或数据库的键。总的来说,优先使用 t.Equal(u) 而不是 t == u,因为 t.Equal() 使用最准确的可比性,并正确处理只有其参数之一具有单调时钟读数的情况。

# Tm.3. **在调用 time.Since(t) 之前,是否*没有*从 t 中剥离单调分量?** 这是前一项的后果。如果在将 time.Time 结构体传递给 time.Since() 函数之前剥离了单调分量(通过调用 UTC()Local()In()Round()Truncate()AddDate()),则 time.Since() 的结果在极少数情况下(例如,如果系统时间在原始获取开始时间与调用 time.Since() 之间通过 NTP 同步)**可能会为负数**。如果*没有*剥离单调分量,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


此内容是 Go Wiki 的一部分。