From 09596d2f54aab08a991dc5f5db272fb2f956f045 Mon Sep 17 00:00:00 2001 From: LTbinglingfeng Date: Wed, 17 Jun 2026 03:19:31 +0800 Subject: [PATCH] Treat loading plugins as busy --- .../api/handlers/management/plugin_store.go | 14 +-- internal/api/handlers/management/plugins.go | 2 +- internal/pluginhost/host.go | 29 ++++- internal/pluginhost/host_test.go | 100 ++++++++++++++++++ 4 files changed, 136 insertions(+), 9 deletions(-) diff --git a/internal/api/handlers/management/plugin_store.go b/internal/api/handlers/management/plugin_store.go index 0217cf5f3..5ea1d8742 100644 --- a/internal/api/handlers/management/plugin_store.go +++ b/internal/api/handlers/management/plugin_store.go @@ -198,15 +198,15 @@ func (h *Handler) installPluginFromStore(c *gin.Context, goos, goarch string) { return } - pluginIsLoaded := func() bool { return pluginLoaded(host, id) } + pluginIsBusy := func() bool { return pluginBusy(host, id) } unloadedBeforeWrite := false result, errInstall := client.Install(installCtx, plugin, pluginstore.InstallOptions{ PluginsDir: pluginsDir, GOOS: goos, GOARCH: goarch, - PluginLoaded: pluginIsLoaded, + PluginLoaded: pluginIsBusy, BeforeWrite: func() error { - if !pluginIsLoaded() { + if !pluginIsBusy() { return nil } if host == nil { @@ -215,8 +215,8 @@ func (h *Handler) installPluginFromStore(c *gin.Context, goos, goarch string) { log.WithFields(log.Fields{ "plugin_id": id, "version": plugin.Version, - }).Info("pluginstore: unloading loaded plugin before install") - if !host.UnloadPlugin(id) && pluginIsLoaded() { + }).Info("pluginstore: unloading busy plugin before install") + if !host.UnloadPlugin(id) && pluginIsBusy() { return pluginstore.ErrLoadedPluginLocked } unloadedBeforeWrite = true @@ -560,9 +560,9 @@ func pluginLocalStatuses(pluginsEnabled bool, pluginsDir string, configs map[str return statuses, nil } -func pluginLoaded(host *pluginhost.Host, id string) bool { +func pluginBusy(host *pluginhost.Host, id string) bool { if host == nil { return false } - return host.PluginLoaded(id) + return host.PluginBusy(id) } diff --git a/internal/api/handlers/management/plugins.go b/internal/api/handlers/management/plugins.go index 3a77d130c..631e61fb6 100644 --- a/internal/api/handlers/management/plugins.go +++ b/internal/api/handlers/management/plugins.go @@ -338,7 +338,7 @@ func (h *Handler) DeletePlugin(c *gin.Context) { return } - if pluginLoaded(host, id) && (host == nil || !host.UnloadPlugin(id)) && pluginLoaded(host, id) { + if pluginBusy(host, id) && (host == nil || !host.UnloadPlugin(id)) && pluginBusy(host, id) { c.JSON(http.StatusConflict, gin.H{ "error": "plugin_delete_requires_restart", "message": "loaded plugin cannot be deleted while the server is running", diff --git a/internal/pluginhost/host.go b/internal/pluginhost/host.go index 83c821525..be52f772f 100644 --- a/internal/pluginhost/host.go +++ b/internal/pluginhost/host.go @@ -39,6 +39,7 @@ type Host struct { mu sync.Mutex loader pluginLoader loaded map[string]*loadedPlugin + loading map[string]struct{} fused map[string]string runtimeConfig *config.Config authManager *coreauth.Manager @@ -65,6 +66,7 @@ func New() *Host { h := &Host{ loader: defaultPluginLoader(), loaded: make(map[string]*loadedPlugin), + loading: make(map[string]struct{}), fused: make(map[string]string), modelClientIDs: make(map[string]struct{}), executorModelClientIDs: make(map[string]struct{}), @@ -137,6 +139,24 @@ func (h *Host) PluginLoaded(id string) bool { return ok } +// PluginBusy reports whether a plugin dynamic library is loaded or being loaded. +func (h *Host) PluginBusy(id string) bool { + if h == nil { + return false + } + id = strings.TrimSpace(id) + if id == "" { + return false + } + h.mu.Lock() + defer h.mu.Unlock() + if _, ok := h.loaded[id]; ok { + return true + } + _, ok := h.loading[id] + return ok +} + func (h *Host) ApplyConfig(ctx context.Context, cfg *config.Config) { if h == nil { return @@ -189,12 +209,18 @@ func (h *Host) ApplyConfig(ctx context.Context, cfg *config.Config) { } if lp == nil { + h.mu.Lock() + h.loading[file.ID] = struct{}{} + h.mu.Unlock() + loaded, errLoad := h.load(file) + h.mu.Lock() + delete(h.loading, file.ID) if errLoad != nil { + h.mu.Unlock() log.Warnf("pluginhost: failed to load plugin %s from %s: %v", file.ID, file.Path, errLoad) continue } - h.mu.Lock() // ApplyConfig, UnloadPlugin, and ShutdownAll are serialized by applyMu, // so a nil read cannot race into a duplicate load. lp = loaded @@ -301,6 +327,7 @@ func (h *Host) ShutdownAll() { }) } h.loaded = make(map[string]*loadedPlugin) + h.loading = make(map[string]struct{}) h.modelClientIDs = make(map[string]struct{}) h.executorModelClientIDs = make(map[string]struct{}) h.modelProviders = make(map[string]string) diff --git a/internal/pluginhost/host_test.go b/internal/pluginhost/host_test.go index df49bd86a..888ac1f78 100644 --- a/internal/pluginhost/host_test.go +++ b/internal/pluginhost/host_test.go @@ -707,6 +707,63 @@ func TestHostApplyConfigSerializesLifecycleCalls(t *testing.T) { } } +func TestHostPluginBusyReportsLoadingPlugin(t *testing.T) { + h, cfg, openStarted, releaseOpen := newBlockingOpenHost(t) + t.Cleanup(h.ShutdownAll) + + applyDone := make(chan struct{}) + go func() { + h.ApplyConfig(context.Background(), cfg) + close(applyDone) + }() + + waitForHostTestSignal(t, openStarted, "plugin open start") + if h.PluginLoaded("alpha") { + t.Fatal("PluginLoaded(alpha) = true, want false while plugin is still loading") + } + if !h.PluginBusy("alpha") { + t.Fatal("PluginBusy(alpha) = false, want true while plugin is loading") + } + + releaseOpen() + waitForHostTestSignal(t, applyDone, "ApplyConfig completion") + if !h.PluginLoaded("alpha") { + t.Fatal("PluginLoaded(alpha) = false, want true after load") + } + if !h.PluginBusy("alpha") { + t.Fatal("PluginBusy(alpha) = false, want true after load") + } +} + +func TestHostUnloadWaitsForBlockingLoad(t *testing.T) { + h, cfg, openStarted, releaseOpen := newBlockingOpenHost(t) + applyDone := make(chan struct{}) + go func() { + h.ApplyConfig(context.Background(), cfg) + close(applyDone) + }() + waitForHostTestSignal(t, openStarted, "plugin open start") + + unloadDone := make(chan bool) + go func() { + unloadDone <- h.UnloadPlugin("alpha") + }() + select { + case <-unloadDone: + t.Fatal("UnloadPlugin completed while ApplyConfig was still loading") + case <-time.After(200 * time.Millisecond): + } + + releaseOpen() + waitForHostTestSignal(t, applyDone, "ApplyConfig completion") + if ok := waitForHostTestBool(t, unloadDone, "UnloadPlugin completion"); !ok { + t.Fatal("UnloadPlugin returned false, want true after loading completes") + } + if h.PluginBusy("alpha") { + t.Fatal("PluginBusy(alpha) = true, want false after unload") + } +} + func TestHostUnloadAndShutdownWaitForBlockingRegister(t *testing.T) { tests := []struct { name string @@ -801,6 +858,49 @@ func (c *capturePluginClient) Call(ctx context.Context, method string, request [ func (c *capturePluginClient) Shutdown() {} +type blockingOpenLoader struct { + inner *testSymbolLoader + started chan struct{} + release <-chan struct{} + startOnce sync.Once +} + +func (l *blockingOpenLoader) Open(file pluginFile, host *Host) (pluginClient, error) { + l.startOnce.Do(func() { close(l.started) }) + <-l.release + return l.inner.Open(file, host) +} + +func newBlockingOpenHost(t *testing.T) (*Host, *config.Config, <-chan struct{}, func()) { + t.Helper() + + inner := newTestSymbolLoader() + plugin := &testPlugin{ + registerResult: validTestPlugin("alpha"), + reconfigureResult: validTestPlugin("alpha"), + } + inner.lookups["alpha"] = newTestSymbolLookup(plugin) + + openStarted := make(chan struct{}) + release := make(chan struct{}) + var releaseOnce sync.Once + releaseOpen := func() { releaseOnce.Do(func() { close(release) }) } + t.Cleanup(releaseOpen) + + h := NewForTest(&blockingOpenLoader{ + inner: inner, + started: openStarted, + release: release, + }) + cfg := &config.Config{ + Plugins: config.PluginsConfig{ + Enabled: true, + Dir: makePluginDir(t, "alpha"), + }, + } + return h, cfg, openStarted, releaseOpen +} + func newBlockingRegisterHost(t *testing.T) (*Host, *config.Config, <-chan struct{}, func()) { t.Helper()