diff --git a/internal/runtime/memory/tidy_dryrun.go b/internal/runtime/memory/tidy_dryrun.go index 14e0dc3..d7c977a 100644 --- a/internal/runtime/memory/tidy_dryrun.go +++ b/internal/runtime/memory/tidy_dryrun.go @@ -142,7 +142,11 @@ func (t *Tidier) logDryRun(out TidyDryRun, ledger *recallLedger) { // 配置关闭(MEMORY_TIDY_ENABLED=false)时如实返回 ErrTidyDisabled:关闭是显式 // 选择,试算不绕过它,也不回一份空候选假装「没有建议」。 func (s *TidyService) DryRun(ctx context.Context) (TidyDryRun, error) { - if !s.cfg.Enabled { + // 走 `enabled()` 而不是裸读字段:这个判断在取 runMu **之前**,而面板调优能在运行期 + // 覆写 `s.cfg`(见 tidy_service_tuning.go 的 ApplyPanelTuning)⇒ 该字段的每个读点 + // 都得与写方同步,否则 `POST …/tidy/dry-run`(含 `?async=1` 那条后台版)与保存并发 + // 时就是数据竞争(`-race` 可复现)。 + if !s.enabled() { return TidyDryRun{}, ErrTidyDisabled } if ctx == nil { diff --git a/internal/runtime/memory/tidy_service.go b/internal/runtime/memory/tidy_service.go index df59f54..c214551 100644 --- a/internal/runtime/memory/tidy_service.go +++ b/internal/runtime/memory/tidy_service.go @@ -122,7 +122,26 @@ func waitTidyTimer(ctx context.Context, d time.Duration) bool { func (s *TidyService) SessionID() string { return TidySessionID } // Enabled 报告整理是否被配置开启(design/23 §4 的 MEMORY_TIDY_ENABLED)。 -func (s *TidyService) Enabled() bool { return s.cfg.Enabled } +// +// 它走 `enabled()` 而不是直接读字段:`Enabled` 是 `s.cfg` 里的一个字段,而面板调优 +// 能在**运行期**整体覆写 `s.cfg`(见 tidy_service_tuning.go 的 ApplyPanelTuning), +// 因此该字段的**每一个**读点都必须与写方同步(`s.mu`)。 +func (s *TidyService) Enabled() bool { return s.enabled() } + +// enabled 在读锁下报告整理开关。它是 `s.cfg.Enabled` 的**唯一**安全读法 +// (口径见 scheduleSnapshot 与上面 Enabled 的注释)。 +func (s *TidyService) enabled() bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.cfg.Enabled +} + +// cfgSnapshot 在读锁下取一份配置快照:凡是不需要 lastRun 的读点用它。 +func (s *TidyService) cfgSnapshot() TidyConfig { + s.mu.Lock() + defer s.mu.Unlock() + return s.cfg +} // Start 启动心跳。它是幂等的:第二次调用是空操作。配置关闭时只记一条日志 // 并立即返回(关闭是显式选择,绝不静默)。ctx(可为 nil)是所有内部工作的 @@ -133,7 +152,7 @@ func (s *TidyService) Enabled() bool { return s.cfg.Enabled } // 整理被配置关掉时清扫也不跑(与 `preinsert_gc.go` 的清扫随审查服务启停同口径), // `Stop` 会一并取消并 join 它。 func (s *TidyService) Start(ctx context.Context) { - if !s.cfg.Enabled { + if !s.enabled() { s.log.Info("memory tidy disabled by configuration", "env", "MEMORY_TIDY_ENABLED") return } @@ -180,16 +199,18 @@ func (s *TidyService) Start(ctx context.Context) { go s.loop(cctx) go s.expirySweepLoop(cctx) + // 启动日志逐项取一次快照(面板调优能在运行期覆写 cfg,因此这些字段也不能裸读)。 + cfg := s.cfgSnapshot() s.log.Info("memory tidy started", - "interval", s.cfg.Interval.String(), - "quiet_interval", s.cfg.QuietInterval.String(), - "quiet_hours", fmt.Sprintf("%02d:00-%02d:00", s.cfg.QuietStartHour, s.cfg.QuietEndHour), - "recall_threshold", s.cfg.RecallThreshold, - "scan_limit", s.cfg.ScanLimit, - "compare_limit", s.cfg.CompareLimit, - "candidate_limit", s.cfg.CandidateLimit, - "auto_apply", s.cfg.autoApplyEnabled(), - "auto_apply_per_round", s.cfg.AutoApplyPerRound, + "interval", cfg.Interval.String(), + "quiet_interval", cfg.QuietInterval.String(), + "quiet_hours", fmt.Sprintf("%02d:00-%02d:00", cfg.QuietStartHour, cfg.QuietEndHour), + "recall_threshold", cfg.RecallThreshold, + "scan_limit", cfg.ScanLimit, + "compare_limit", cfg.CompareLimit, + "candidate_limit", cfg.CandidateLimit, + "auto_apply", cfg.autoApplyEnabled(), + "auto_apply_per_round", cfg.AutoApplyPerRound, "rule_version", TidyRuleVersion, "session_id", TidySessionID, "last_run_at", s.lastRunLabel()) @@ -277,7 +298,7 @@ func (s *TidyService) RunOnce(ctx context.Context) (TidyStats, error) { // 由 tidy_auto.go 直接落地,其余仍等人点(候选 apply / 按 run 批量 apply)。 // 关掉自动档只需 `$DATA_DIR/config/memory-tidy.json` 的 `{"autoApply": false}`。 func (s *TidyService) ManualRun(ctx context.Context) (TidyStats, error) { - if !s.cfg.Enabled { + if !s.enabled() { return TidyStats{}, ErrTidyDisabled } return s.runRound(ctx, "manual") diff --git a/internal/runtime/memory/tidy_service_tuning.go b/internal/runtime/memory/tidy_service_tuning.go index b21d0ce..c25ba08 100644 --- a/internal/runtime/memory/tidy_service_tuning.go +++ b/internal/runtime/memory/tidy_service_tuning.go @@ -47,10 +47,12 @@ import ( // (交叉调用 Start/Stop 在 `-race` 下 `panic: sync: WaitGroup is reused before previous // Wait has returned`)。唤醒通道把这两件事一起消掉:**调优路径上一个 Start/Stop 都没有**。 // -// 并发口径:写 `s.cfg` 与调度读(`nextDelay` / `dueNow`)由 `s.mu` 互斥 -// (调度读走 `scheduleSnapshot`,与本次改动的同一把锁下取同一版本);写 `tidier.cfg` -// 与**所有轮次**由 `s.runMu` 互斥(心跳轮、手动轮、试算都持它)。因此这里只需要 -// 「持 runMu 写 → 发一次唤醒」,不与任何既有锁序冲突。 +// 并发口径:`s.cfg` 由 `s.mu` 互斥 —— 🔴 **按字段扫,不按文件扫**:这个字段既然会被 +// 运行期整体覆写,它的**每一个读点**都必须与写方同步,因此所有读点都收进了持锁访问器 +// (`enabled()` / `cfgSnapshot()` / `scheduleSnapshot()`:调度读 `nextDelay` / +// `dueNow`、两个活端点的开关判定 `DryRun` / `ManualRun`、以及 `Enabled()` 与 `Start` +// 的启动日志)。写 `tidier.cfg` 则与**所有轮次**由 `s.runMu` 互斥(心跳轮、手动轮、 +// 试算都持它)。因此这里只剩下「持两把锁写 → 发一次唤醒」,不与任何既有锁序冲突。 // ApplyPanelTuning 把面板值当场叠到运行期(见文件头三条口径)。 // diff --git a/internal/runtime/memory/tidy_service_tuning_test.go b/internal/runtime/memory/tidy_service_tuning_test.go index 293e892..22d78df 100644 --- a/internal/runtime/memory/tidy_service_tuning_test.go +++ b/internal/runtime/memory/tidy_service_tuning_test.go @@ -2,6 +2,7 @@ package memory import ( "context" + "errors" "strings" "sync" "testing" @@ -401,15 +402,18 @@ func TestPreInsertReviewer_PanelTuningRaceWithGateReads(t *testing.T) { // TestTidyService_PanelTuningRaceWithLiveHeartbeat 是整理档的**并发回归哨兵**。 // -// 背景(实测踩出来的,不是推演):`s.cfg` 的**调度读**(`nextDelay` / `dueNow`) -// 不持锁,因此「面板调优写 cfg」必须落在一个**没有任何 goroutine 读它**的窗口里。 -// 曾经的顺序 `Stop → Start → 写` 在这里会被 -race 抓住:新起的循环立刻读 `s.cfg` -// 重算下一拍,与写入并排(而且那一拍还可能按**旧**配置算出来)。正确顺序是 -// `Stop → 写 → Start`。 +// 背景(实测踩出来的,不是推演):面板调优会在运行期**整体覆写** `s.cfg`,因此该字段 +// 的**每一个**读点都必须与写方同步。心跳那一路的读点(`nextDelay` / `dueNow`)现在走 +// `scheduleSnapshot()`(同一把锁下同取「cfg + lastRun」)—— 这条哨兵钉的就是它:早先 +// 那版把写入放在 `Start` **之后**,新起的循环立刻读 `s.cfg` 重算下一拍,与写入并排 +// (而且那一拍还可能按**旧**配置算出来),`-race` 会当场报出来。 // // 读者刻意**不**另起 goroutine 调 `nextDelay()`:那个入口在包里只有心跳 goroutine // 会调(生产代码不在别处调它),另起一个就是伪造一个生产不存在的调用方。这里直接让 -// **真心跳**当读者(连跑多轮,每轮都要停一次、起一次),与生产形态一致。 +// **真心跳**当读者(连跑多轮,每轮都换一次周期),与生产形态一致。 +// +// 与它同族的另两条哨兵:审查档(`PreInsertReviewer` 的并发读写)与试算/手动跑那两个 +// **活端点**(`TestTidyService_PanelTuningRaceWithDryRunGuard`)。 // // 同样**只在 `-race` 下有意义**(不带 -race 时它是一次并发冒烟 + 值生效断言)。跑法: // @@ -440,3 +444,55 @@ func TestTidyService_PanelTuningRaceWithLiveHeartbeat(t *testing.T) { t.Fatalf("最后一轮的 intervalSec = %d, want %d", svc.cfg.IntervalSec, want) } } + +// TestTidyService_PanelTuningRaceWithDryRunGuard 是**第三条**并发回归哨兵 +// (前两条:审查档的真并发读写、整理档的真心跳):面板调优覆写 `s.cfg` 与 +// **试算端点的开关判定**并发。 +// +// 为什么单独要这一条:`TidyService.DryRun` 的 `if !s.cfg.Enabled` 发生在**取 runMu +// 之前**,因此它不受 runMu 保护 —— 而 `POST /admin/memories/tidy/dry-run`(含 +// `?async=1` 那条会起后台 goroutine、真机上跑 90s 级的版本)正是这条读点的活端点。 +// 只把「调度读」收进持锁访问器是不够的:**同一个字段的每一个读点**都要一起收。 +// +// 它**只在 `-race` 下有意义**(不带 -race 时是一次并发冒烟 + 结果断言)。跑法: +// +// go test -race -count=1 -run TestTidyService_PanelTuningRaceWithDryRunGuard ./internal/runtime/memory/ +func TestTidyService_PanelTuningRaceWithDryRunGuard(t *testing.T) { + base := TidyConfig{Enabled: true, IntervalSec: 86400, QuietStartHour: 23, QuietEndHour: 7} + svc := newTidyServiceForTest(t, newTestStore(t), base, nil, time.Now, nil) + + const rounds = 40 + var wg sync.WaitGroup + wg.Add(2) + // 写方:模拟面板保存(每次换一个周期,一定走整体覆写 s.cfg)。 + go func() { + defer wg.Done() + for i := 0; i < rounds; i++ { + panel := runtimeconfig.MemoryTidyTuning{IntervalSec: tuningInt(3600 + i)} + if err := svc.ApplyPanelTuning(base, panel); err != nil { + t.Errorf("ApplyPanelTuning: %v", err) + return + } + } + }() + // 读方:两个**活端点**交替 —— 试算(POST …/tidy/dry-run)与手动真跑 + // (POST …/tidy/run)。两者的开关判定都在取 runMu **之前**,因此都在这个字段上 + // 与写方交手;只覆盖其中一个就等于漏了另一个。 + go func() { + defer wg.Done() + for i := 0; i < rounds; i++ { + if i%2 == 0 { + if _, err := svc.DryRun(context.Background()); err != nil && !errors.Is(err, ErrTidyDisabled) { + t.Errorf("DryRun: %v", err) + return + } + continue + } + if _, err := svc.ManualRun(context.Background()); err != nil && !errors.Is(err, ErrTidyDisabled) { + t.Errorf("ManualRun: %v", err) + return + } + } + }() + wg.Wait() +} diff --git a/internal/services/gateway/admin_memory_tuning_file.go b/internal/services/gateway/admin_memory_tuning_file.go index ade22a0..9776bc7 100644 --- a/internal/services/gateway/admin_memory_tuning_file.go +++ b/internal/services/gateway/admin_memory_tuning_file.go @@ -227,14 +227,17 @@ func tuningUnitLabel(spec memoryTidyTuningSpec) string { } } +// tuningValueClipRunes 是错误文案里回显「收到的值」的 rune 上限。 +const tuningValueClipRunes = 40 + // clipJSON 把收到的原始 JSON 片段截断成可安全回显的短串(错值可能很长)。 +// +// 🔴 按 **rune** 截断,**不按字节**:值里完全可能是中文(主人就爱填中文),按字节切 +// 会把一个多字节字符切成两半 —— 回显里因此出现半截 UTF-8(JSON 编码后变成 U+FFFD +// 替换符),而这条文案的用途正是让人**看清自己填错了什么**。手法与同包的 +// `clipRunes`(notifications.go)同源,直接复用而不是再写一份。 func clipJSON(raw json.RawMessage) string { - const maxClip = 40 - s := strings.TrimSpace(string(raw)) - if len(s) <= maxClip { - return s - } - return s[:maxClip] + "…" + return clipRunes(strings.TrimSpace(string(raw)), tuningValueClipRunes) } // isJSONNull 报告这段原始值是不是 JSON 的 `null`(清除哨兵)。 diff --git a/internal/services/gateway/admin_memory_tuning_guard_test.go b/internal/services/gateway/admin_memory_tuning_guard_test.go index c5e9e3e..24aa56a 100644 --- a/internal/services/gateway/admin_memory_tuning_guard_test.go +++ b/internal/services/gateway/admin_memory_tuning_guard_test.go @@ -2,11 +2,14 @@ package gateway import ( "context" + "encoding/json" "errors" "net/http" "os" + "strings" "testing" "time" + "unicode/utf8" "git.yeij.top/AskaEth/Cyrene_v2/internal/auth" "git.yeij.top/AskaEth/Cyrene_v2/internal/config" @@ -261,3 +264,52 @@ func TestAdminMemoryTidyTuning_UnavailableWithoutDataDir(t *testing.T) { t.Fatal("没有数据目录时不该解析出落点") } } + +// TestAdminMemoryTidyTuningWrite_LongValueEchoIsRuneSafe 「非法值」的文案会把**收到的 +// 值**回显出来(这是它的用途:让人看清自己填错了什么),而主人完全可能填中文 —— +// 回显必须按 rune 截断:按**字节**切会把一个汉字切成半截(非法 UTF-8,经 JSON 编码后 +// 变成 U+FFFD 替换符),等于把「你填了什么」显示成乱码。 +// +// 取值刻意让截断点**落在多字节字符中间**:JSON 值自带引号(1 字节),再让首个字符是 +// 1 字节 ASCII、其余是 3 字节汉字 ⇒ 「引号 + a」占 2 字节,第 40 字节正好切在第 13 个 +// 汉字的**第三个字节**上。若只堆汉字(引号 1 + 3n 字节),40 会恰好落在 rune 边界上, +// 那样这条用例会**假绿**(实测过:按字节截断也照样通过)。 +func TestAdminMemoryTidyTuningWrite_LongValueEchoIsRuneSafe(t *testing.T) { + t.Parallel() + + // ① 纯函数层:截断结果必须是合法 UTF-8(这一条与 JSON 层无关,最直接)。 + long := json.RawMessage(`"a` + strings.Repeat("三", 60) + `"`) + clipped := clipJSON(long) + if !utf8.ValidString(clipped) { + t.Fatalf("clipJSON 切出了非法 UTF-8:%q", clipped) + } + if strings.ContainsRune(clipped, utf8.RuneError) { + t.Fatalf("clipJSON 结果里有替换符:%q", clipped) + } + + // ② 端点层:400 文案里不许出现替换符,且开头几个字仍要如实回显。 + env := newTuneEnv(t, Deps{}) + before := env.seed(t, `{"intervalSec": 86400}`) + payload := `a` + strings.Repeat("三", 60) + resp, body := env.put(t, env.token(t, auth.RoleOwner), `{"quietStartHour": "`+payload+`"}`) + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body=%s)", resp.StatusCode, body) + } + assertErrorCode(t, body, "invalid_payload") + var eobj struct { + Code string `json:"code"` + Message string `json:"message"` + } + if err := json.Unmarshal(decodeBody(t, body)["error"], &eobj); err != nil { + t.Fatalf("decode error: %v (body=%s)", err, body) + } + if !utf8.ValidString(eobj.Message) || strings.ContainsRune(eobj.Message, utf8.RuneError) { + t.Fatalf("回显里出现了替换符/非法 UTF-8(半个汉字被切了):%q", eobj.Message) + } + if !strings.Contains(eobj.Message, "a三三三") { + t.Fatalf("文案应当仍回显开头几个字:%q", eobj.Message) + } + if after := env.bytes(t); string(after) != string(before) { + t.Fatalf("非法输入改动了盘上内容:\nbefore=%s\nafter=%s", before, after) + } +}