发现一个 golang 结构体字段被异常修改的问题，大家帮我看看

V2EX = way to explore

V2EX 是一个关于分享和探索的地方

现在注册

已注册用户请登录

The Go Programming Language

› http://golang.org/

› Go Playground

Go Projects

› Revel Web Framework

这是一个创建于 649 天前的主题，其中的信息可能已经有所发展或是发生改变。

简单描述，就是在内存保存数据，在创建和查询过程中，某些字段的值会在查询时意外的被改变，改变的方式也很奇怪。

例如存在一个结构体 Task 和一个全局变量 list:

var list sync.Map
type Task struct {
    ID int64
    Name string
    User string
}

创建并把 task 保存在全局变量 list 中；

task := Task {
    ID: now.UnixMicro(),
    Name: "agent-web",
    User: "wangds",
}
list.Store(task.Name, task)

执行查询时，task 的值可会意外的改变，发生概率盲猜有 0.1-0.4 ；而且每次更改代码后，只遵循以下 5 种改变模式中的 1 种：

{
    ID: 1677200690411702,
    Name: "agent-web",
    User: "agent-",
}
{
    ID: 1677200690411702,
    Name: "1gent-web",
    User: "wangds",
}
{
    ID: 1677200690411702,
    Name: "agent-web",
    User: "1angds",
}
{
    ID: 1677200690411702,
    Name: "agent-web",
    User: "167720",
}
{
    ID: 1677200690411702,
    Name: "167720069",
    User: "wangds",
}

全局变量试过其他类型，比如 map 、slice ，还试过一个第三方的内存缓存工具 ristretto ，都有这个问题。

https://gitee.com/tianshuapp/web-deploy-task-manage

第 1 条附言 · 2023-02-24 14:16:48 +08:00

破案了，代码增加了 gin 框架模式，在 gin 下就正常，在 fiber 下就异常。
感谢大家的帮助！

第 2 条附言 · 2023-02-24 14:51:04 +08:00

gofiber 框架加 Immutable 配置后也正常了
app := fiber.New(fiber.Config{
Immutable: true,
})

40 条回复 • 2023-03-06 23:30:27 +08:00

rrfeng

2023-02-24 11:45:32 +08:00

逻辑都没写全，怎么判断哪里有问题……

pathletboy

2023-02-24 11:46:22 +08:00

所有更新数据的地方打 LOG 嘛，很快就能找到。

Wangds

2023-02-24 11:48:31 +08:00

@rrfeng 文章底部有代码，可以复现

Wangds

2023-02-24 11:49:39 +08:00

@pathletboy 有在协程里持续打印，发现是查询的一瞬间改变的，但是不知道为什么会改变

dcalsky

2023-02-24 11:49:53 +08:00

帮你看看 != 帮你 review 整个项目，你发个 repo 的链接是要闹哪样

john2022

2023-02-24 11:50:07 +08:00

使用内存地址而不是值试试

Wangds

2023-02-24 11:52:09 +08:00

@dcalsky 不是整个项目啊，是创建和查询的最小实现。

Wangds

2023-02-24 11:53:02 +08:00

@john2022 我再试试，当时好像也试过指针

john2022

2023-02-24 11:56:17 +08:00

另外，你这个 list 和 list2 是 package 私有变量，不是全局变量

john2022

2023-02-24 11:57:33 +08:00

全局变量最好使用 func init 来初始化，并且最好用大写的，比如 TaskList TaskList1 ，对 list 的修改使用锁，否则有可能会被清除

Maboroshii

2023-02-24 12:03:37 +08:00

你的 map key 为什么是 Name 而不是 ID ？

Wangds

2023-02-24 12:05:12 +08:00

@Maboroshii 我记得用 ID 也会变

Wangds

2023-02-24 12:06:38 +08:00

@john2022 是要把 list 和 list2 放到 main 包里吗

anerevol

2023-02-24 12:09:46 +08:00

你这创建 task 的时候，判断同名的 task 是否存和创建 task 不是原子操作吧

john2022

2023-02-24 12:11:13 +08:00

model 里创建 func init(){
}
在 main 里面使用 import _ "web-deploy-task-manage/model"

john2022

2023-02-24 12:11:59 +08:00

@anerevol 应该是线程不安全，所以要使用读写锁

Wangds

2023-02-24 12:12:25 +08:00

@anerevol 以前有个版本是加了锁的，也有这个问题。我给代码加个延时试试

echoless

2023-02-24 12:13:44 +08:00 via Android

老弟问题能不能在一个文件里面复现

Wangds

2023-02-24 12:15:55 +08:00

@anerevol 创建方法里查，判断同名代码后加了个延时；测试的代码里每次创建、查询、循环之间都加了延时；
肉眼可看的一个一个蹦日志，也会出现问题，哭了

Wangds

2023-02-24 12:16:22 +08:00

@wuhaoecho 我试试

anerevol

2023-02-24 12:16:32 +08:00

@Wangds #17 你加锁所有写操作的地方都要加锁

Wangds

2023-02-24 12:16:55 +08:00

@john2022 我试试

Wangds

2023-02-24 12:17:59 +08:00

@anerevol 是的，当时是读、写都加了锁，当时用的 sync.RWMutex

joshu

2023-02-24 12:19:21 +08:00

在 model 写个能复现问题的单元测试吧，实在看不懂什么叫能复现

Aoang

2023-02-24 12:24:04 +08:00

看了看，楼上说了的，包级别的全局变量最好通过 Init() 来初始化。

还有 sync.Map 适用的场景你怕不是根本就没思考过，你这么写，最起码也得用读写锁 + map

用 map 来管理，我看你还有更新值的操作，你不存指针，你想怎么更新 map 里面的值？

你这一通操作，*Task 是不安全的，把你的 map 加好锁吧。读写锁估计都没用，你几个方法都有写操作

- https://gitee.com/tianshuapp/web-deploy-task-manage/blob/master/services/task.go#L19-39
改成一个方法 GetOrCreate ，内部加锁

model 下面的方法加锁。不要想着先读取，所以加一个读写锁，读完了释放。然后再加写锁，去更新。
这期间，你的 *Task 都变了。。。

还有返回全部内容的方法，返回的数据是不能有指针的，除非和上面一样加锁。

Wangds

2023-02-24 12:49:17 +08:00

我下午再优化改一下，感觉受益良多

kiwi95

2023-02-24 12:57:30 +08:00

如果是数据竞争导致的，写个单测， `go test -race` 很容易看出来

Wangds

2023-02-24 13:25:28 +08:00

@kiwi95 哇塞，我去看看

Wangds

2023-02-24 13:50:45 +08:00

更新了一下：
不再缓存指针了；
代码放到单文件里了，init 函数初始化全局变量；
map 的 key 改为 id ；
担心 id 太长，现在从 1 自增；
加了读写锁，且测试加了延时；
现在代码精简了，创建请求只涉及创建，没有查询了；目前只有创建、查询两种请求操作；
通过`go run -race main.go`来执行程序，没有报任何异常；
字段异常修改的问题依然存在。

我在 main 方法的协程里直接测试，就一切正常，请求通过 gofiber 就会有问题。

Wangds

2023-02-24 14:16:38 +08:00

破案了，代码增加了 gin 框架模式，在 gin 下就正常，在 fiber 下就异常。
感谢大家的帮助！

virusdefender

2023-02-24 14:18:28 +08:00

go run -race 然后并发测试下看看，可能是有竞争之类的

Wangds

2023-02-24 14:28:38 +08:00

@virusdefender 我试试

liuxu

2023-02-24 14:30:24 +08:00

fiber 的 Context 会复用，见 fiber 文档首页“Zero Allocation”章节，https://docs.gofiber.io/#zero-allocation

你从*fiber.Ctx 拿数据的时候得 memory copy ，https://gitee.com/tianshuapp/web-deploy-task-manage/blob/master/main.go#L98

user := c.Query("user", "anonymous")
arch := c.Query("arch", "")
改成
user := utils.CopyString(c.Query("user", "anonymous"))
arch := utils.CopyString(c.Query("arch", ""))

或者 fiber 全局配置添加
app := fiber.New(fiber.Config{
Immutable: true,
})

Wangds

2023-02-24 14:41:07 +08:00

@virusdefender 并发下确实会报 DATA RACE ，我看看楼下的方法

Wangds

2023-02-24 14:45:25 +08:00

@liuxu 我试试

Wangds

2023-02-24 14:49:56 +08:00

@liuxu 加了 Immutable: true 正常了，拜谢！！

echoless

2023-02-24 14:53:23 +08:00 via Android

@liuxu 怪不得有人不推荐 fiber 优化玩的太狠了

anerevol

2023-02-24 15:22:09 +08:00

task := Task{
ID: idCounter,
//ID: 1677200690411702,
Name: strings.Clone(name),
User: strings.Clone(user),
Stats: StatRunning,
Message: "",
Arch: strings.Clone(arch),
CreateTime: &now,
UpdateTime: nil,
DoneTime: nil,
Expires: expires,
Deleted: false,
} debug 了下，虽然没去看 fiber ，结论是一样的。其实是和 string 的实现有关

lucarfulllll

2023-02-27 11:46:49 +08:00

看了下例子，有点不懂的地方想问下楼主和留言的大神。
sync.map{} 按照官方的描述就是并发安全的，而且内部实现也是加了 Mutex 锁，为啥请求中还加了读写锁呢？麻烦指教

var rwLock sync.RWMutex

// mode=2
var List2 sync.Map

.....

// GetTaskByIDModel 查询 task
func GetTaskByIDModel(id int64) (Task, error) {
var task Task
var ok bool
rwLock.RLock()
defer rwLock.RUnlock(). // 此处还加读写锁是否多余呢？
if mode == 1 {
task, ok = List[id]
} else if mode == 2 {
v, o := List2.Load(id)
if o {
task, ok = v.(Task)
if !ok {
return Task{}, errors.New("not found")
}
} else {
log.Println("从 sync.Map 中获取 task 失败")
}
}
if !ok {
return Task{}, errors.New("not found")
}
return task, nil
}

Wangds

2023-03-06 23:30:27 +08:00

@lucarfulllll 我感觉应该不用再加锁了