GitHub CLI 中 attestation 测试的数据竞争问题分析

背景介绍

在 GitHub CLI 项目的测试过程中，开发团队发现了一个数据竞争(data race)问题。这个问题出现在 attestation 功能模块的测试代码中，具体是在模拟 HTTP 客户端的行为时发生的并发访问冲突。

问题现象

测试运行时会出现如下警告信息：

WARNING: DATA RACE
Read at 0x00c000440ae0 by goroutine 72:
  github.com/cli/cli/v2/pkg/cmd/attestation/api.(*failAfterNCallsHttpClient).Get()
  ...
Previous write at 0x00c000440ae0 by goroutine 73:
  github.com/cli/cli/v2/pkg/cmd/attestation/api.(*failAfterNCallsHttpClient).Get()
  ...

这表明有两个 goroutine 同时访问了同一个内存地址，一个在读取，一个在写入，形成了数据竞争。

技术分析

根本原因

问题的根源在于测试代码中使用的模拟 HTTP 客户端 failAfterNCallsHttpClient 的实现。这个客户端有一个计数器 NumCalls 用于记录调用次数，但这个计数器在并发环境下没有进行任何同步保护。

并发场景

在测试 TestFetchBundleFromAttestations_FailOnTheSecondAttestation 时，代码会并发地调用多个 HTTP 请求。每个请求都会通过 failAfterNCallsHttpClient 的 Get 方法，该方法会对 NumCalls 进行递增操作。

数据竞争的具体表现

当多个 goroutine 同时调用 Get 方法时，它们会同时尝试读取和修改 NumCalls 的值。由于 Go 语言中的递增操作不是原子性的（它实际上是读取-修改-写入三个步骤的组合），这就导致了数据竞争。

解决方案

使用互斥锁保护

最直接的解决方案是使用 sync.Mutex 来保护对 NumCalls 的访问。在每次访问计数器前加锁，操作完成后解锁，确保同一时间只有一个 goroutine 能够修改计数器。

使用原子操作

另一种更轻量级的解决方案是使用 sync/atomic 包提供的原子操作。可以将 NumCalls 的类型改为 int32 或 int64，然后使用 atomic.AddInt32 或 atomic.AddInt64 来进行原子递增。

重现问题的方法

虽然这个问题在常规测试中不一定每次都能重现，但可以通过以下命令增加测试次数来更容易地触发数据竞争：

go test -v -count=128 -run 'TestFetchBundleFromAttestations_FailOnTheSecondAttestation' -race ./pkg/cmmd/attestation/api/...

经验教训

1. 并发环境下的共享资源：任何在并发环境下被共享的变量都需要考虑同步问题，即使是在测试代码中也不例外。

2. 测试代码的质量：测试代码同样需要遵循生产代码的质量标准，特别是在并发处理方面。

3. Go 的竞争检测器：Go 语言内置的竞争检测器(-race标志)是发现这类问题的强大工具，应该在持续集成中始终启用。

结论

这个数据竞争问题虽然出现在测试代码中，但它提醒我们在编写任何并发代码时都需要特别注意共享资源的访问控制。通过适当的同步机制，可以确保代码在并发环境下的正确性和稳定性。