From 8f9e2669a68b3cf32677b3c3f94b87198494880d Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 30 Apr 2021 19:46:57 +0530 Subject: [PATCH] MM-34419: Fix plugin enable race (#17511) A race happens when we try to enable plugins in an HA environment. There is no loss in functionality here, but it's a timing bug. There are two bugs here. The first one is a case of nested config listeners. a.InitPlugins is called in InitServer, and as well as a config listener. And that calls a.SyncPluginsActiveState(). But inside a.InitPlugins, there is yet another config listener which again calls a.SyncPluginsActiveState(). The first fix is to simply not call the method again from the nested listener. The second bug happens because the config changed message is sent across the cluster only after saving the config locally in the store. To fix this, we simply change GetPluginStatus to not fail and return empty status when other nodes don't have plugins enabled. https://mattermost.atlassian.net/browse/MM-34419 ```release-note Fix a race condition where enabling plugins would result in spurious errors in the logs. ``` --- app/plugin.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/plugin.go b/app/plugin.go index 0c678977f8..3df5147247 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -210,9 +210,13 @@ func (a *App) InitPlugins(pluginDir, webappPluginDir string) { // Sync plugin active state when config changes. Also notify plugins. a.Srv().PluginsLock.Lock() a.RemoveConfigListener(a.Srv().PluginConfigListenerId) - a.Srv().PluginConfigListenerId = a.AddConfigListener(func(*model.Config, *model.Config) { - a.installFeatureFlagPlugins() - a.SyncPluginsActiveState() + a.Srv().PluginConfigListenerId = a.AddConfigListener(func(old, new *model.Config) { + // If plugin status remains unchanged, only then run this. + // Because (*App).InitPlugins is already run as a config change hook. + if *old.PluginSettings.Enable == *new.PluginSettings.Enable { + a.installFeatureFlagPlugins() + a.SyncPluginsActiveState() + } if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { if err := hooks.OnConfigurationChange(); err != nil {