mirror of
https://gitcode.com/JianFeeeee/HomeAgent.git
synced 2026-11-06 01:07:35 +00:00
fix(plugin): 修两处持锁自死锁(CI 双红真因)+ tools_ready 事件声明
## 死锁(这是 CI 连续两次红的真因) `StopAll` 与 `StopPlugin` 都在持有 `r.mu` **写锁**时调用 `RemoveBuiltinSkill` → `skillDirOf` → `r.mu.RLock()` ⇒ **自死锁**。 后果不是报错而是**卡死**: - internal/plugins 卡 300s 超时(TestCapability_RestrictedPluginStillLoads) - internal/plugin 卡 840s 超时(TestStopPluginDoesNotTouchPersistentState) - CI 的 `go test ./...` 与 `go test -race` 双双红 修法:拆出**不加锁**的内部版本,持锁路径只走它: - `skillDirFrom(dataDir, name)`(纯函数) - `removeBuiltinSkillLocked(pluginName, p, dataDir)`(显式传 dataDir, 避免"看起来没取锁、一改就取锁"的隐患) - `announceSkillRemovedLocked`(持锁下直读 r.evBus) 修后:internal/plugins 300s → 12s,internal/plugin 840s → 0.7s。 ## 防回归判据 `TestStopAllAndStopPluginDoNotDeadlock`:死锁**不返回**,普通断言抓不到, 所以用**超时**当判据(5 秒未返回即失败),并指明应改用 Locked 版本。 ## tools_ready 事件声明 新增的 tools_ready 触发 `TestEveryPublishedEventHasAnOutwardPath` (该判据防"事件发了、客户端收不到"的静默失效)。 tools_ready 是**内核内部**就绪信号(plugin-host → seq 等订阅者), 不是对话流程事件,前端无需感知 ⇒ 在 allowed 里显式声明理由。 ## 验证 go build ./... / go vet ./... / go test -p 1 ./... / -race 全通过。
This commit is contained in:
@ -61,6 +61,18 @@ func (r *Registry) skillDirOf(skillName string) string {
|
||||
r.mu.RLock()
|
||||
dataDir := r.dataDir
|
||||
r.mu.RUnlock()
|
||||
return skillDirFrom(dataDir, skillName)
|
||||
}
|
||||
|
||||
// skillDirFrom 是 skillDirOf 的**不加锁**版本(纯函数)。
|
||||
//
|
||||
// ★ 为什么必须拆出来:StopAll 持有 r.mu **写锁**时调用 RemoveBuiltinSkill,
|
||||
// 而 skillDirOf 要取 RLock ⇒ 自死锁。实测后果:internal/plugins 的
|
||||
// TestCapability_RestrictedPluginStillLoads 卡在 cleanup 300s 超时,
|
||||
// CI 的 go test ./... 与 -race 双双红(2026-10-25)。
|
||||
//
|
||||
// 规则:已持锁的调用路径只能用不加锁版本。
|
||||
func skillDirFrom(dataDir, skillName string) string {
|
||||
if dataDir == "" {
|
||||
return ""
|
||||
}
|
||||
@ -122,12 +134,25 @@ func (r *Registry) InstallBuiltinSkill(pluginName string, p sdk.Plugin) {
|
||||
// 而工具已被停用 —— 模型会拿到一连串"工具不存在"的错误,
|
||||
// 且它无从知道原因是"插件被关了"。
|
||||
func (r *Registry) RemoveBuiltinSkill(pluginName string, p sdk.Plugin) {
|
||||
r.mu.RLock()
|
||||
dataDir := r.dataDir
|
||||
r.mu.RUnlock()
|
||||
r.removeBuiltinSkillLocked(pluginName, p, dataDir)
|
||||
}
|
||||
|
||||
// removeBuiltinSkillLocked 是 RemoveBuiltinSkill 的**不加锁**版本。
|
||||
//
|
||||
// 调用方必须已持有 r.mu(且不能再调任何会取锁的方法)。
|
||||
// 由 StopAll 使用 —— 那里持写锁,走加锁版会自死锁。
|
||||
// 显式传入 dataDir 而不是在里面读 r.dataDir:避免"看起来没取锁、
|
||||
// 实际上一改就取锁"的隐患。
|
||||
func (r *Registry) removeBuiltinSkillLocked(pluginName string, p sdk.Plugin, dataDir string) {
|
||||
provider, ok := p.(SkillProvider)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
for name := range provider.BuiltinSkills() {
|
||||
dir := r.skillDirOf(name)
|
||||
dir := skillDirFrom(dataDir, name)
|
||||
if dir == "" {
|
||||
continue
|
||||
}
|
||||
@ -137,7 +162,7 @@ func (r *Registry) RemoveBuiltinSkill(pluginName string, p sdk.Plugin) {
|
||||
}
|
||||
log.Printf("[plugin] %s: removed skill %q (plugin is stopped)", pluginName, name)
|
||||
// 通知 skillmgr 从索引里摘掉(复用同一事件,负载带 removed 标记)。
|
||||
r.announceSkillRemoved(pluginName, dir)
|
||||
r.announceSkillRemovedLocked(pluginName, dir)
|
||||
}
|
||||
}
|
||||
|
||||
@ -164,10 +189,9 @@ func (r *Registry) announceSkill(pluginName, dir string) {
|
||||
})
|
||||
}
|
||||
|
||||
func (r *Registry) announceSkillRemoved(pluginName, dir string) {
|
||||
r.mu.RLock()
|
||||
// announceSkillRemovedLocked 发布"skill 已移除"(**不加锁**,供持锁路径调用)。
|
||||
func (r *Registry) announceSkillRemovedLocked(pluginName, dir string) {
|
||||
bus := r.evBus
|
||||
r.mu.RUnlock()
|
||||
if bus == nil {
|
||||
return
|
||||
}
|
||||
|
||||
@ -15,6 +15,7 @@ import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/JianFeeeee/HologramAgent/internal/events"
|
||||
sdk "github.com/JianFeeeee/HologramAgent/internal/sdk"
|
||||
@ -190,3 +191,42 @@ func TestBuiltinSkill_AnnouncesRemoved(t *testing.T) {
|
||||
t.Fatal("卸载通告必须带 removed=true(否则 skillmgr 分不清是新增还是移除)")
|
||||
}
|
||||
}
|
||||
|
||||
// ── 持锁路径不得调用加锁版(防自死锁)──
|
||||
//
|
||||
// 真实事故(2026-10-25):StopAll 与 StopPlugin 都在持有 r.mu 写锁时
|
||||
// 调 RemoveBuiltinSkill → skillDirOf → r.mu.RLock() ⇒ **自死锁**。
|
||||
//
|
||||
// 后果不是报错而是**卡死**:internal/plugins 卡 300s、internal/plugin 卡 840s
|
||||
// (go test 默认 10m/20m 超时才死),CI 的 go test ./... 与 -race 双双红。
|
||||
//
|
||||
// 死锁无法用普通断言抓(它不返回),所以这里用**超时**当判据:
|
||||
// 正常应毫秒级返回,超过 5 秒即视为死锁。
|
||||
func TestStopAllAndStopPluginDoNotDeadlock(t *testing.T) {
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
|
||||
// ① StopAll 路径
|
||||
r1 := newTestRegistryForSkills(t)
|
||||
p1 := &fakeSkillPlugin{name: "demo", skills: map[string]string{"demo-tools": sampleSkill}}
|
||||
r1.InstallBuiltinSkill("demo", p1)
|
||||
r1.StopAll()
|
||||
|
||||
// ② StopPlugin 路径(需要真的把插件注册进 r.plugins)
|
||||
r2 := newTestRegistryForSkills(t)
|
||||
p2 := &fakeSkillPlugin{name: "demo", skills: map[string]string{"demo-tools": sampleSkill}}
|
||||
r2.InstallBuiltinSkill("demo", p2)
|
||||
r2.mu.Lock()
|
||||
r2.plugins["demo"] = p2
|
||||
r2.mu.Unlock()
|
||||
_ = r2.StopPlugin("demo")
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("StopAll/StopPlugin 超时 —— 极可能是持锁调用了加锁版 " +
|
||||
"RemoveBuiltinSkill(自死锁)。请改用 removeBuiltinSkillLocked。")
|
||||
}
|
||||
}
|
||||
|
||||
@ -147,7 +147,7 @@ func buildPluginWithRealTemplate(t *testing.T, businessCode string) string {
|
||||
t.Fatalf("解析 SDK 路径: %v", err)
|
||||
}
|
||||
mustWriteFile(t, filepath.Join(dir, "go.mod"),
|
||||
"module e2eplugin\n\ngo 1.25\n\n"+
|
||||
"module e2eplugin\n\ngo 1.27.1\n\n"+
|
||||
"require github.com/JianFeeeee/hologramagentsdk v0.9.2\n\n"+
|
||||
"replace github.com/JianFeeeee/hologramagentsdk => "+sdkPath+"\n")
|
||||
|
||||
|
||||
@ -1084,8 +1084,12 @@ func (r *Registry) StopAll() {
|
||||
}
|
||||
// 关停时摘掉所有随包 skill:进程要退出,留下 skill 会让下次启动
|
||||
// 出现"索引里有、插件却没起"的错位(尤其插件被配置禁用时)。
|
||||
//
|
||||
// ★ 必须用**不加锁**的路径:此处 r.mu 是写锁,而 RemoveBuiltinSkill →
|
||||
// skillDirOf 要取 RLock ⇒ 自死锁(实测 300s 卡死,CI 双红)。
|
||||
// 直接读 r.dataDir(已在锁内)并调纯函数 skillDirFrom。
|
||||
for _, st := range stoppers {
|
||||
r.RemoveBuiltinSkill(st.plugin.Name(), st.plugin)
|
||||
r.removeBuiltinSkillLocked(st.plugin.Name(), st.plugin, r.dataDir)
|
||||
}
|
||||
r.plugins = make(map[string]sdk.Plugin)
|
||||
r.instances = nil
|
||||
@ -1575,7 +1579,11 @@ func (r *Registry) StopPlugin(name string) error {
|
||||
}
|
||||
// 先摘 skill 再停插件:停用后模型若仍看到 skill,会照做并拿到
|
||||
// 一连串"工具不存在",且无从知道原因是插件被关了。
|
||||
r.RemoveBuiltinSkill(name, p)
|
||||
//
|
||||
// ★ 用**不加锁**版:此处 r.mu 是写锁(上面刚 Lock 过),
|
||||
// 而 RemoveBuiltinSkill 会取 RLock ⇒ 自死锁。
|
||||
// 与 StopAll 是同一个坑(实测两处各卡 300s / 90s 超时)。
|
||||
r.removeBuiltinSkillLocked(name, p, r.dataDir)
|
||||
|
||||
r.runStopHandlers(name)
|
||||
stopErr := p.Stop()
|
||||
|
||||
Reference in New Issue
Block a user