fix(config): owner_lane.json 并发落盘不再损坏/丢更新(review p1)
review p1:Set 的固定 tmp 名(path+".tmp")+ 落盘发生在 mu.Unlock() 之后 ⇒ 并发 Set 撞同一临时文件把 JSON 写坏(load 退回默认「甲」、主人刚说的「少说点」 白说且无提示),或 rename 完成顺序与新快照先后不一致而丢更新。 修法(两条都取,各司其职): - 落盘挪进锁内(Set 全程持 s.mu):并发 Set 完全串行 ⇒ 快照与 rename 顺序一致, 不可能丢更新。落盘是一个小 JSON,持锁 I/O 代价可忽略;读侧读内存、不读文件。 - 每次用 os.CreateTemp 新建临时文件(仓库同款先例 internal/user/save.go), 不再撞固定 tmp 名;CreateTemp 以 0600 打开并再 chmod 一次,任一步失败都清掉 tmp。 - 旧 save(快照) 换成 saveLocked()(调用方须持锁,直接 marshal s.cfg)。 测试:新增并发用例 TestOwnerLaneStoreConcurrentSetKeepsAllUpdates(24 轮 × 24 写者), 断言 ① 文件始终是合法 JSON、② 每一次成功的 Set 都能在最终文件读到(不丢更新)。 修前红(原实现在同一用例上):5/5 轮复现,两种失败模式都抓到 —— "并发落盘把配置写坏了(文件不是合法 JSON):invalid character '\"' after top-level value" 与 "丢更新 —— 键 qq-obv11|g-N-M 不在最终文件里";修后连续 3 次绿,-race 下亦 ok。 Bridge.SetOwnerLanePolicy 无需改动:它在 ownerLaneMu 下设内存后调注入的 store, 串行化由 store 侧保证。
This commit is contained in:
1 parent
1d49d266f0
commit
48a5bb89f2
2 files changed
+92
-16
No files matched your search
@@ -138,10 +138,17 @@ func (s *OwnerLaneStore) Snapshot() OwnerLaneFile {
|
||||
}
|
||||
|
||||
// Set 写入某个 (platform, groupID) 的策略并落盘。groupID 为空表示写全局 default 档。
|
||||
// 内存先改(行为立即生效),再尽力落盘;落盘失败返回错误(调用方留可见 WARN),
|
||||
// 但内存里的新档位仍然生效 —— 「改不动磁盘」不该让已经做出的行为决定回退。
|
||||
//
|
||||
// 内存改动与**落盘都在同一把锁内**(review p1):并发 Set 因此完全串行 —— 既不会两个
|
||||
// 写者去写同一个临时文件把 JSON 写坏,也不会因为原子 rename 的完成先后与各快照的新旧
|
||||
// 不一致而丢掉「后发生的那次更新」。落盘量是一个小 JSON,持锁期间的 I/O 代价可忽略;
|
||||
// 读侧(PolicyFor)读的是内存、不碰文件,因此不会被这次落盘抖动。
|
||||
//
|
||||
// 内存先改、失败返回错误(调用方留可见 WARN),但内存里的新档位仍然生效 ——
|
||||
// 「改不动磁盘」不该让已经做出的行为决定回退。
|
||||
func (s *OwnerLaneStore) Set(platform, groupID string, p OwnerLanePolicy) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
if groupID == "" {
|
||||
s.cfg.Default = p
|
||||
} else {
|
||||
@@ -150,20 +157,18 @@ func (s *OwnerLaneStore) Set(platform, groupID string, p OwnerLanePolicy) error
|
||||
}
|
||||
s.cfg.Groups[OwnerLaneGroupKey(platform, groupID)] = p
|
||||
}
|
||||
snapshot := OwnerLaneFile{Default: s.cfg.Default, Groups: make(map[string]OwnerLanePolicy, len(s.cfg.Groups))}
|
||||
for k, v := range s.cfg.Groups {
|
||||
snapshot.Groups[k] = v
|
||||
}
|
||||
s.mu.Unlock()
|
||||
|
||||
if err := s.save(snapshot); err != nil {
|
||||
if err := s.saveLocked(); err != nil {
|
||||
return fmt.Errorf("runtimeconfig: save owner lane config %s: %w", s.path, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// save 原子写出配置(临时文件 + rename),使并发读者永远不会看到半份文件。
|
||||
func (s *OwnerLaneStore) save(f OwnerLaneFile) error {
|
||||
// saveLocked 原子写出当前配置。调用方**必须持有 s.mu**。
|
||||
//
|
||||
// 它每次都用**新建的临时文件**(`os.CreateTemp`,仓库同款先例见 internal/user/save.go)
|
||||
// 再原子 rename:即便有别的进程/goroutine 也在写同一个目录,也不会撞同一个临时文件名,
|
||||
// 而半份文件永远不会以 s.path 的名字出现。
|
||||
func (s *OwnerLaneStore) saveLocked() error {
|
||||
if s.path == "" {
|
||||
return nil
|
||||
}
|
||||
@@ -173,15 +178,33 @@ func (s *OwnerLaneStore) save(f OwnerLaneFile) error {
|
||||
return fmt.Errorf("mkdir: %w", err)
|
||||
}
|
||||
}
|
||||
raw, err := json.MarshalIndent(f, "", " ")
|
||||
raw, err := json.MarshalIndent(s.cfg, "", " ")
|
||||
if err != nil {
|
||||
return fmt.Errorf("marshal: %w", err)
|
||||
}
|
||||
tmp := s.path + ".tmp"
|
||||
if err := os.WriteFile(tmp, raw, 0o600); err != nil {
|
||||
return fmt.Errorf("write %s: %w", tmp, err)
|
||||
tmp, err := os.CreateTemp(dir, filepath.Base(s.path)+".*.tmp")
|
||||
if err != nil {
|
||||
return fmt.Errorf("create temp in %s: %w", dir, err)
|
||||
}
|
||||
if err := os.Rename(tmp, s.path); err != nil {
|
||||
tmpName := tmp.Name()
|
||||
// CreateTemp 以 0600 打开;显式再 chmod 一次,使落盘文件不因 umask 变化而更宽。
|
||||
// 任一步失败都删掉临时文件,绝不留垃圾。
|
||||
if err := tmp.Chmod(0o600); err != nil {
|
||||
_ = tmp.Close()
|
||||
_ = os.Remove(tmpName)
|
||||
return fmt.Errorf("chmod %s: %w", tmpName, err)
|
||||
}
|
||||
if _, err := tmp.Write(raw); err != nil {
|
||||
_ = tmp.Close()
|
||||
_ = os.Remove(tmpName)
|
||||
return fmt.Errorf("write %s: %w", tmpName, err)
|
||||
}
|
||||
if err := tmp.Close(); err != nil {
|
||||
_ = os.Remove(tmpName)
|
||||
return fmt.Errorf("close %s: %w", tmpName, err)
|
||||
}
|
||||
if err := os.Rename(tmpName, s.path); err != nil {
|
||||
_ = os.Remove(tmpName)
|
||||
return fmt.Errorf("rename: %w", err)
|
||||
}
|
||||
return nil
|
||||
|
||||
Reference in new issue
Block a user